Re: [PATCH v2 1/2] push: add "--[no-]force-if-includes"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 12, 2020, 18:20 UTC
- Message-ID
- <xmqq5z8ioo15.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20200912150459.8282-2-shrinidhi.kaushik@gmail.com>
Srinidhi Kaushik <shrinidhi.kaushik@gmail.com> writes:
Show 23 quoted lines
> Add a new option: `--force-if-includes` to `git-push` where forced > updates are allowed only if the tip of the remote-tracking ref has > been integrated locally, by verifying if the tip of the remote-tracking > ref on which a local branch has based on (for a rewrite), is reachable > from at least one of the `reflog` entries of the local branch about > to be updated by force on the remote. > > This option can also be used with `--force-with-lease` in setups > where the remote-tracking refs of the repository are implicitly > updated in the background. > > If a local branch is based on a remote ref for a rewrite, and if that > remote-tracking ref is updated by a push from another repository after > it has been checked out locally, force updating that branch to remote > with `--force-with-lease[=<refname>[:expect]]` without specifying the > "<refname>" or "<expect>" values, can cause the update that happened > in-between the checkout and forced push to be lost. > > Specifying `--force-with-includes` with `--force-with-lease` as an > ancillary argument at the time of push, ensures that any new updates > to the remote-tracking refs are integrated locally before allowing a > forced update. This behavior can enabled by default if the configuration > option `push.forceIfIncludesWithLease` is set to `true`.
This step seems to do too many things at once. Split into logical progression of improvements, or nobody can sensibly review it, I am afraid.
Show 16 quoted lines
> diff --git a/Documentation/config/push.txt b/Documentation/config/push.txt > index f5e5b38c68..1b4948faa0 100644 > --- a/Documentation/config/push.txt > +++ b/Documentation/config/push.txt > @@ -114,3 +114,11 @@ push.recurseSubmodules:: > specifying '--recurse-submodules=check|on-demand|no'. > If not set, 'no' is used by default, unless 'submodule.recurse' is > set (in which case a 'true' value means 'on-demand'). > + > +push.forceIfIncludesWithLease:: > + If set to `true`, adds `--force-if-includes` as an ancillary argument > + to `--force-with-lease[=<refname>[:<expect>]]`, when "<refname>" or > + "<expect>" values are unspecified at the time of push. > ++ > +Note: Specifying `--no-force-if-includes` to linkgit:git-push[1] as an > +argument during the time of push does _not_ override this configuration.
I do not see why you still want to link these two unrelated features. I may want to do forced push with lease when I know I am rewinding, and I may want to do a if-included force when needed, but I do not see why I want to ask the former and implicitly see it also trigger the latter.
I haven't seen the details, but if we severe the (unnecessary?) entanglement of these two features, perhaps this patch will become a lot smaller and more focused? I dunno.