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

Git rebase detection #57651

Merged
merged 3 commits into from Sep 13, 2018

Conversation

Projects
None yet
2 participants
@jgbright
Contributor

jgbright commented Aug 31, 2018

I'm not sure how sensitive vscode needs to be about file i/o. The bug here is that rebase detection was unaware that if the rebase directory is missing, there is no rebase in progress. I added a couple of async fs.exists calls parallel to the existing code that reads the rebase commit hash. If we think it is far more common that the user is not dealing with a rebase collision, we may want to optimize around that. We could wait until after successfully reading the hash before checking for the rebase directories. I believe that would make it slightly slower in the less common case in which a rebase is happening.

A completely different option would be to read the commit hash from one of the rebase folders and forego the REBASE_HEAD file.

@jgbright

This comment has been minimized.

Show comment
Hide comment
@jgbright

jgbright Aug 31, 2018

Contributor

I plan to squash this, but I thought seeing the test setup script might be helpful for review.

Contributor

jgbright commented Aug 31, 2018

I plan to squash this, but I thought seeing the test setup script might be helpful for review.

@jgbright

This comment has been minimized.

Show comment
Hide comment
@jgbright

jgbright Aug 31, 2018

Contributor

Fixes #57299

Contributor

jgbright commented Aug 31, 2018

Fixes #57299

@joaomoreno joaomoreno added this to the Backlog milestone Aug 31, 2018

@joaomoreno joaomoreno added the git label Aug 31, 2018

@joaomoreno joaomoreno merged commit 86230c2 into Microsoft:master Sep 13, 2018

2 checks passed

VS Code #20180831.1 succeeded with issues
Details
license/cla All CLA requirements met.

@joaomoreno joaomoreno modified the milestones: Backlog, September 2018 Sep 13, 2018

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Sep 13, 2018

Member

Thanks! 🍻

Member

joaomoreno commented Sep 13, 2018

Thanks! 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment