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

don't check for changed files when checking out a new branch #829

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

ianhi
Copy link
Collaborator

@ianhi ianhi commented Nov 19, 2020

Checking out a new branch will never cause files to change so we don't need to to check changed files. The attempt to check was failing because we were asking for the git diff between the current branch and one that did not yet exist

Fixes: #828

This should never cause files to change, and was failing because we were asking for the git diff between the current branch and one that did not yet exist
@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch ianhi/jupyterlab-git/issue828

@ianhi ianhi requested a review from fcollonval November 19, 2020 21:39
@ianhi ianhi mentioned this pull request Nov 19, 2020
@fcollonval
Copy link
Member

@ianhi Thanks a lot for the quick fix.

Would you have time to add unit tests on this feature and the one addressed by #824 to reduce potential troubles in the future?

it would make sense to put them in https://github.com/jupyterlab/jupyterlab-git/blob/master/tests/model.spec.tsx#L299. I think you could assert internal calls using spy like https://github.com/jupyterlab/jupyterlab-git/blob/master/tests/model.spec.tsx#L365.

@ianhi
Copy link
Collaborator Author

ianhi commented Nov 22, 2020

Will do. Thanks for the pointers on how to do it, makes it much more approachable.

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

LGTM

I merge this one to cut a patch release. But if you have time to add some tests in another PR, that would be much appreciate.

@fcollonval fcollonval merged commit 3b6613a into jupyterlab:master Nov 26, 2020
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.

Creating a new branch
2 participants