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

Add git stashing #29138

Merged
merged 4 commits into from
Aug 15, 2017
Merged

Add git stashing #29138

merged 4 commits into from
Aug 15, 2017

Conversation

Krzysztof-Cieslak
Copy link
Contributor

Addresses #1904 which is one of the oldest and most upvoted issues.

Adds 3 new commands for git integration:

  • git stash
  • git stash pop using latest stash
  • git stash pop with choosing stash

@msftclas
Copy link

@Krzysztof-Cieslak,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@Krzysztof-Cieslak
Copy link
Contributor Author

CC: @joaomoreno

@joaomoreno
Copy link
Member

joaomoreno commented Aug 11, 2017

@Krzysztof-Cieslak We don't really need to have the stashes in the model itself. This will just make git status slower since we'd need to call git once more in order to get the stashes. You can just make this call once one of the stash actions runs. Can you fix that?

@Krzysztof-Cieslak
Copy link
Contributor Author

Yes, I'll rebase and fix it.

@Krzysztof-Cieslak
Copy link
Contributor Author

Ready to review again.

@joaomoreno
Copy link
Member

joaomoreno commented Aug 15, 2017

@Krzysztof-Cieslak

These changes are not formatted and violate some linting rules we have set. We have configured a git pre-commit hook to validate these for you, which gets set up when you run ./scripts/npm.sh install. You were only able to commit by not running that or by commenting out the pre-commit hook. Next time, please commit while running through these rules. I've fixed those issues for you on this one.

Also, this goes into infinite recursion. Next time, please run your changes before handing them over to us.

Also, this condition will fail when index is zero.

Also, I've added functionality to provide a stash message when stashing changes.

I've fixed all that up.

We do appreciate your contributions very much. But next time, try to follow the guidelines better so we make this smoother.

@joaomoreno joaomoreno merged commit 8dcb181 into microsoft:master Aug 15, 2017
@Krzysztof-Cieslak
Copy link
Contributor Author

Sorry for the trouble @joaomoreno, I haven't paid enough attention to this PR, indeed. My bad :(

@Krzysztof-Cieslak Krzysztof-Cieslak deleted the stashing branch August 15, 2017 20:35
@joaomoreno joaomoreno modified the milestones: Backlog, August 2017 Sep 13, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants