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

Compare current branch to target - don't assume already on target branch locally #141

Merged
merged 1 commit into from Oct 3, 2020

Conversation

danlester
Copy link
Contributor

Otherwise switching branches doesn't work as per issue #124

This just uses the git syntax:

git log ..origin/branchname

instead of the old

git log branchname..origin/branchname

In the old case, this failed when the local repo wasn't already on branchname:

fatal: ambiguous argument 'branchname..origin/branchname': unknown revision or path not in the working tree.

Otherwise switching branches doesn't work as per issue jupyterhub#124
@welcome
Copy link

welcome bot commented Jul 9, 2020

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

Copy link
Member

@manics manics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable to me. Can anyone else foresee any problems?

@manics manics linked an issue Oct 3, 2020 that may be closed by this pull request
Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function find_upstream_changed where this is run, is called once to detect the added files in comparison to the local and current branch instead of the local branch of the same name.

I'm not 100%, but I think it can make sense to compare the current local branch rather than the local repos branch of the same name..

If I follow the logic, the find_upstream_changed is called from rename_local_untracked which is modifying the local files without changing branch in between. With that said, I conclude this is a clear improvement of the logic as it becomes at least consistent with the idea to change the current branch, as compared to inconsistent with both the logic of switching branch first and changing that.

@consideRatio consideRatio merged commit 0857c28 into jupyterhub:master Oct 3, 2020
@welcome
Copy link

welcome bot commented Oct 3, 2020

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@consideRatio
Copy link
Member

Thank you @jhgoodwin, @danlester, and @manics for reporting, suggesting a resolution, and reviewing it! ❤️ 🎉

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

Successfully merging this pull request may close these issues.

Fails to switch branches properly
3 participants