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 support for force push and force-with-lease #53286

Merged
merged 1 commit into from Sep 13, 2018

Conversation

Projects
None yet
5 participants
@nesukun
Contributor

nesukun commented Jun 29, 2018

This PR introduces support for force pushing with and without lease.

Force pushing is disabled by default, and if enabled, it will use force-with-lease unless set otherwise. This is to deter people that aren't aware of the consequences that the action might have. There is also a confirmation dialog which can be configured to never show if the user wants. Separate command variants with force are added to differentiate them from regular pushing, but those will not show if the option is not set to allow it.

Made this PR before realising the work made in #53170 but I think this provides enough additional value to be considered over that one.

Please be aware that English is not my primary language, so even if I made my best effort to avoid typos, I might have missed some.

This fixes #53045

@msftclas

This comment has been minimized.

Show comment
Hide comment
@msftclas

msftclas Jun 29, 2018

CLA assistant check
All CLA requirements met.

msftclas commented Jun 29, 2018

CLA assistant check
All CLA requirements met.

@nesukun

This comment has been minimized.

Show comment
Hide comment
@nesukun

nesukun Jul 4, 2018

Contributor

@joaomoreno Please let me know if you need anything from me or I can help in any way to push this forward.

Contributor

nesukun commented Jul 4, 2018

@joaomoreno Please let me know if you need anything from me or I can help in any way to push this forward.

@joaomoreno joaomoreno added this to the Backlog milestone Jul 5, 2018

@joaomoreno joaomoreno added the git label Jul 5, 2018

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Jul 5, 2018

Member

@nesukun All good so far

Member

joaomoreno commented Jul 5, 2018

@nesukun All good so far

@nesukun

This comment has been minimized.

Show comment
Hide comment
@nesukun

nesukun Jul 6, 2018

Contributor

Resolved conflicts, sorry for the noise

Contributor

nesukun commented Jul 6, 2018

Resolved conflicts, sorry for the noise

@nesukun

This comment has been minimized.

Show comment
Hide comment
@nesukun

nesukun Aug 3, 2018

Contributor

Resolved conflicts :)

Contributor

nesukun commented Aug 3, 2018

Resolved conflicts :)

@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

Great job! Thanks! 🍻

Member

joaomoreno commented Sep 13, 2018

Great job! Thanks! 🍻

@joaomoreno joaomoreno merged commit 907a021 into Microsoft:master Sep 13, 2018

1 of 2 checks passed

VS Code #20180908.16 failed
Details
license/cla All CLA requirements met.
Details
@qkdreyer

This comment has been minimized.

Show comment
Hide comment
@qkdreyer

qkdreyer Oct 9, 2018

Using Code 1.28.0 (431ef9d) I'm unable to see this new pushForce from the "More actions..." context menu.

image

qkdreyer commented Oct 9, 2018

Using Code 1.28.0 (431ef9d) I'm unable to see this new pushForce from the "More actions..." context menu.

image

@nesukun

This comment has been minimized.

Show comment
Hide comment
@nesukun

nesukun Oct 9, 2018

Contributor

@qkdreyer unfortunately, the command is only available through the command palette as it was considered a 'dangerous' or 'advanced' operation. I'll create an additional PR to get the option added there and let the maintainers decide wether or not it should appear there.

Contributor

nesukun commented Oct 9, 2018

@qkdreyer unfortunately, the command is only available through the command palette as it was considered a 'dangerous' or 'advanced' operation. I'll create an additional PR to get the option added there and let the maintainers decide wether or not it should appear there.

@styfle

This comment has been minimized.

Show comment
Hide comment
@styfle

styfle Oct 9, 2018

@nesukun Nice PR! I just saw this in the release notes.

I wanted to point out that there might be a couple ways to avoid making a mistake for the GUI users.

  1. Use force-with-lease since it's almost always a better choice then plain force. Probably don't even need a GUI option for plain force.
  2. Avoid showing force-with-lease for the default/master branch and only show this GUI option for non-default branches. These branches are less likely to be shared among multiple people when compared to the default branch and thus, will be less dangerous to force push.
  3. Avoid force-with-lease in the GUI if the branch has not diverged

styfle commented Oct 9, 2018

@nesukun Nice PR! I just saw this in the release notes.

I wanted to point out that there might be a couple ways to avoid making a mistake for the GUI users.

  1. Use force-with-lease since it's almost always a better choice then plain force. Probably don't even need a GUI option for plain force.
  2. Avoid showing force-with-lease for the default/master branch and only show this GUI option for non-default branches. These branches are less likely to be shared among multiple people when compared to the default branch and thus, will be less dangerous to force push.
  3. Avoid force-with-lease in the GUI if the branch has not diverged
@qkdreyer

This comment has been minimized.

Show comment
Hide comment
@qkdreyer

qkdreyer Oct 9, 2018

@nesukun Since it's a dangerous/advanced operation, we could let a user decide to display it in the "scm/title" menus with another option disabled by default.

qkdreyer commented Oct 9, 2018

@nesukun Since it's a dangerous/advanced operation, we could let a user decide to display it in the "scm/title" menus with another option disabled by default.

@joaomoreno

This comment has been minimized.

Show comment
Hide comment
@joaomoreno

joaomoreno Oct 9, 2018

Member

Why not use the same option to control whether the actions appear in the menu?

Member

joaomoreno commented Oct 9, 2018

Why not use the same option to control whether the actions appear in the menu?

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