-
Notifications
You must be signed in to change notification settings - Fork 31.9k
Add support for force push and force-with-lease #53286
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
Conversation
c32d40b
to
586b9d0
Compare
@joaomoreno Please let me know if you need anything from me or I can help in any way to push this forward. |
@nesukun All good so far |
586b9d0
to
b7618b7
Compare
Resolved conflicts, sorry for the noise |
b7618b7
to
c64c26d
Compare
Resolved conflicts :) |
c64c26d
to
907a021
Compare
Great job! Thanks! 🍻 |
Using Code 1.28.0 (431ef9d) I'm unable to see this new pushForce from the "More actions..." context menu. |
@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. |
@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.
|
@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. |
Why not use the same option to control whether the actions appear in the menu? |
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