Re: [PATCH v2 1/2] push: add "--[no-]force-if-includes"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Srinidhi Kaushik <shrinidhi.kaushik@xxxxxxxxx> writes:

> 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.

> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux