-
-
Notifications
You must be signed in to change notification settings - Fork 487
Add @effect/sql-pglite package #4692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next-minor
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: cb5dfd6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
02d328c to
1ea11b9
Compare
|
Not sure why the lint and docgen failed, they pass on my branch locally. |
4cba33b to
5c9e640
Compare
f265f41 to
0fd314b
Compare
|
Sorry for taking forever to get back to this. I've been extremely busy. I cleaned everything up, addressed the review comments, rebased on main, and updated everything to match latest changes in sql. @tim-smart if you would take another look I'd appreciate it! |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, I'm not sure how that happened! Thanks for pointing it out, I'll try to clean it up, although I'm not sure of the best way to do so. Maybe I should copy the pglite changes to a new branch and open a new PR that's clean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm yeah IDK what happened here. My history looks clean locally. The commits relating to pglite are clean. It seems that the most recent 5 commits from main are somehow included in the PR after I rebased. I'm not sure why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because your PR is opened against next-minor branch, and should have been rebased against it. git rebase -i interactive mode is very helpful in such situations, and you can easily remove a few commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. I rebased from main but there are commits there that aren't in next-minor yet so they got picked up. I'm not sure what the right thing is here. Should I make a new PR with the pglite stuff on top of next-minor instead of main?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to make a new branch. Just make git rebase -i HEAD~12 and inside the interactive editor, delete rows with commits you don't want
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah got it, fixed
c07a3f1 to
cb5dfd6
Compare
f752fc3 to
2bedcf6
Compare
24ae504 to
6bc6da5
Compare
a2457a5 to
51a27ea
Compare
7922be1 to
8fa279a
Compare
Type
Description
Add @effect/sql-pglite package. An @effect/sql driver for PGlite, single user postgresql in the browser or node.
Related