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 force with lease to the advanced push options #2991

Merged
merged 2 commits into from Aug 9, 2016

Conversation

EbenZhang
Copy link
Contributor

@EbenZhang EbenZhang commented Oct 21, 2015

Fixes #2955

forcewithlease

@pmiossec
Copy link
Member

I really like this option to be merge into GitExtensions. To tell you my life, today is the first time I really mess the central repository at work.
After spending quite some time helping a colleague on his computer, I come back to mine to continue what I did. I force push with GitExtensions what I thought was my personal feature branch. But in fact, I was in the master branch , just to realize what I did (3 commits lost but finally recovered!) :(

If GitExtensions would have the --force-with-lease option, it would have prevented me to do this mistake. So, I will be really pleased to see this PR merged.

Thanks @EbenZhang (for this PR and for the others!)

But is it possible to do 2 things :

  • switch the place of the 2 options. I think that "force-with-lease" should be the first under the mouse i.e. the first presented because that's the less dangerous otherwise people will check the "force" because they just know this one
  • put tooltips to explain the 2 options because a very few know this option (and that's the one that should be preferred!)

@EbenZhang
Copy link
Contributor Author

@pmiossec Your suggestion sounds great!

@EbenZhang EbenZhang force-pushed the EbenForceWithLease branch 2 times, most recently from c27028b to adbde97 Compare October 30, 2015 12:48
private readonly TranslationString _useForceWithLeaseInstead =
new TranslationString("Force push may overwrite other people's change. Do you want to use the safer force with lease instead?");
private readonly TranslationString _forceWithLeaseTooltips =
new TranslationString("Force with lease is a safer way to force push, it ensures you don't overwrite other's work.");

Choose a reason for hiding this comment

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

Maybe "Force with lease is a safer way to force push. It ensures you only overwrite work that you have seen in your local repository."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yours sounds more accurate.

@SeanFeldman
Copy link

Out of curiosity, this has is ready for merge, but never happened. What's preventing this to be merged?
Thank you.

@EbenZhang
Copy link
Contributor Author

I think they only want to merge bug fixes not features. Maybe we should have a branch for features.

@vbjay
Copy link
Contributor

vbjay commented Mar 11, 2016

https://github.com/gitextensions/gitextensions/wiki/Project-Workflow

On Thu, Mar 10, 2016 at 8:48 PM EbenZhang notifications@github.com wrote:

I think they only want to merge bug fixes not features. Maybe we should
have a branch for features.


Reply to this email directly or view it on GitHub
#2991 (comment)
.

@SeanFeldman
Copy link

@EbenZhang looks like you've done everything except the last step: a screenshot :)

@vbjay is that it?

@KindDragon
Copy link
Contributor

I don't like two checkboxes for force push, because of that I add it only to push popup dialog.

@pmiossec
Copy link
Member

pmiossec commented Apr 26, 2016 via email

@vbjay
Copy link
Contributor

vbjay commented Apr 26, 2016

I agree. If they are pushing tags then alert them and get their consent to
do a force and then ask are you sure dialog. If not, use lease.

On Tue, Apr 26, 2016 at 6:23 AM Philippe Miossec notifications@github.com
wrote:

Yes but because of that, it's a feature hard to discover! I only found out
the day I made a mistake.

I think, I highly prefer to show 'force-with-lease' by default instead of
'force' to prevent mistakes.

And propose force only if it fails (It could be an idea to never propose it
because I don't see where 'force' could be useful over 'force-with-lease'.
And in these exceptional cases, we could fall back to command line) .

My point of view is that 'force-with-lease' is a lot better practice than
'force' and for that, it should be put forward and be the default way to do
a "force"...


You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#2991 (comment)

@EbenZhang
Copy link
Contributor Author

rebased on top of the latest master to solve conflicts

@Do-do-new
Copy link
Contributor

Would really like to see it merged, along with left-panel.
Thanks @EbenZhang !

@jbialobr jbialobr merged commit 5e24353 into gitextensions:master Aug 9, 2016
@EbenZhang EbenZhang deleted the EbenForceWithLease branch August 9, 2016 14:33
@SeanFeldman
Copy link

Is there a way to specify that the default for push in GitExtensions should be --force-with-lease? Relying on setting manually a checkbox is not very reliable.

@pmiossec
Copy link
Member

@SeanFeldman Are you sure you want to set --force-with-lease the default option when you push?
It makes no sense for me because if you ever fetched the commits of other developper, you could end overwriting the remote history and loose history (which is ironic because this option was introduced with the goal to not loose history inadvertently!)

@vbjay
Copy link
Contributor

vbjay commented Feb 15, 2017 via email

@GrahamTheCoder
Copy link

I think the thing still missing is from the dialog that pops up from a normal conflicting push - which offers only a force push, not force with lease iirc

@pmiossec
Copy link
Member

pmiossec commented Feb 16, 2017 via email

@SeanFeldman
Copy link

@pmiossec what I meant is to have a single "force" option, and to be able to configure what's done by default when and if push is force. SourceTree does it that way. They allow to configure the default behavior for forced push, either a regular --force or --force-with-lease.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants