-
-
Notifications
You must be signed in to change notification settings - Fork 535
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
Migrate dolt checkout
to new CLI framework.
#6209
Conversation
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.
Looking good. A couple small style/flow suggestions. Ping me again when the existing and new bats tests are passing.
…and `assertTargetBranchClean`
9ae8b72
to
1ce8051
Compare
1ce8051
to
9ee4da1
Compare
…e they get caught when moving changes to the target branch.
9ee4da1
to
bb32bbb
Compare
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.
Main concern is that we need more tests for testing the state of the current branch when switching between remote and local mode. I'm not convinced that the current local-remote.bash approach covers this well enough.
go/libraries/doltcore/sqle/dprocedures/dolt_checkout_helpers.go
Outdated
Show resolved
Hide resolved
a94789f
to
9af1a71
Compare
…local commands use the branch specified in repo_state.json as the default branch for the session.
3ee76db
to
ec7f29b
Compare
8ea54b7
to
5de290e
Compare
…hem once local commands use the branch specified in repo_state.json as the default branch for the session.
9d01739
to
4dfbce2
Compare
99b73b6
to
9251a89
Compare
…o nicktobey/dolt-checkout
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.
Ship when the tests pass!
. o ..
o . o o.o
...oo
__[]__
__|_o_o_o\__
\""""""""""/
\. .. . /
^^^^^^^^^^^^^^^^^^^^
This PR migrates dolt branch to invoke SQL commands instead of manipulating the database directly. This allows it to work even on remote connections.
We implement this by adding an additional flag to the
dolt_checkout
stored proceudre:--global
. This tells the SQL environment to persist this checkout for subsequent connections, making it the new default branch for future connections.If a server is currently running,
dolt_checkout("--global")
will return an error. This is because we try to avoid changing the default branch on a server until we can properly consider the indented effect on existing connections.Because working sets behave differently in our SQL environment than the command line (command line has a single working set, SQL land has a working set per-branch), we only permit changing the default branch when there would be no observable different in behavior: that is, when both the former and the new branch are clean. If this isn't the case, we return an error. This guarentees that a user calling
dolt checkout
from the command line will never see behavior that differs from the original CLI implementation. They will, at worst, get a helpful error message.