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

Manually drop successful stash pop with conflicts #7432

Merged
merged 6 commits into from
May 1, 2019

Conversation

outofambit
Copy link
Contributor

@outofambit outofambit commented Apr 29, 2019

Overview

Closes #7383

Description

  • adds --quiet option to stash pop
  • adds 1 as a success exit code for stash pop and manually inspects for output
  • manually drops stash when its popped but not dropped by git
  • updates and adds more test coverage for stash pop

@outofambit outofambit added this to the 1.7.0 milestone Apr 29, 2019
app/src/lib/git/stash.ts Outdated Show resolved Hide resolved
@tierninho
Copy link
Contributor

For testing this, just want to check that this fix only pertains to 1) when there are conflicts in the Changes list, and 2) a stash is popped successfully. Therefore we should drop the stash as expected and the files will remain in the Changes list with the conflicted files?

pop [--index] [-q|--quiet] [<stash>] Remove a single stashed state from the stash list and apply it on top of the current working tree state.

Any changes to the stash list I should look out for and/or is it relevant for testing? Thanks!

@outofambit
Copy link
Contributor Author

For testing this, just want to check that this fix only pertains to 1) when there are conflicts in the Changes list, and 2) a stash is popped successfully. Therefore we should drop the stash as expected and the files will remain in the Changes list with the conflicted files?

@tierninho this does not apply to situations where there are changes in the working directory. this is only for when you have made commits after making a stash. If those commits touch the same files/lines of any files/lines in the stash, then this comes into play. The expected result is to have the stash popped and dropped.

@outofambit outofambit force-pushed the bugfix/stash-pop-conflicts-drop branch from 4a2882b to 1b90c4b Compare April 29, 2019 22:28
@outofambit outofambit added the ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 30, 2019
Copy link
Member

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

Just one question, everything else looks fine to me

app/test/unit/git/stash-test.ts Show resolved Hide resolved
@shiftkey shiftkey self-assigned this Apr 30, 2019
@shiftkey shiftkey requested a review from tierninho April 30, 2019 13:09
Copy link
Member

@shiftkey shiftkey left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@tierninho tierninho left a comment

Choose a reason for hiding this comment

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

LGTM ⚡️

@shiftkey shiftkey merged commit a0ec9e1 into development May 1, 2019
@shiftkey shiftkey deleted the bugfix/stash-pop-conflicts-drop branch May 1, 2019 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[stashing] successfully popping a stash with conflicts doesn't drop the stash
3 participants