Skip to content
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

Deal with modify/delete conflicts #269

Merged
merged 5 commits into from
Jun 15, 2022

Conversation

jdmansour
Copy link
Contributor

This pull request deals with the case where you have diverged branches - you delete a file remotely, and edit the same file locally, or vice versa. If the local change was committed previously, the sync will fail with CONFLICT (modify/delete). See also #265.

puller: $ git -c user.email=nbgitpuller@nbgitpuller.link -c user.name=nbgitpuller merge -Xours origin/master
puller: CONFLICT (modify/delete): README.md deleted in origin/master and modified in HEAD. Version HEAD of README.md left in tree.
puller: Automatic merge failed; fix conflicts and then commit the result.

What this does is, it checks for that error, and resolves it via commit -a (keeping all the local changes). This should be in line with the sync policy. We have tested this for about a week in production and so far it seems to work well.

Please let me know if I'm adding too many tests :-) I think maybe I could squash some together if it is getting too much.

Instead, move the commit step into the merge function,
as it is not called anywhere else
@yuvipanda
Copy link
Contributor

@jdmansour omg no such thing as too many tests! thank you so much for this, and apologies it took a while for me to get to it.

I've pushed a change that stops using Exceptions for flow control, but otherwise this Looks Great to me. I'll leave this open for a day or two to see if you (or others) have objections to my changes, but if not I'll merge!

@jdmansour
Copy link
Contributor Author

@yuvipanda No problem, and thanks for getting back to this! Your change looks good, the original code was trying to be a bit too clever with the exception.

@yuvipanda yuvipanda merged commit 6b9a3e6 into jupyterhub:main Jun 15, 2022
@yuvipanda
Copy link
Contributor

Thanks a lot, @jdmansour!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants