Re: [PATCH v2 04/23] contrib/subtree: Teach push and pull to use .gittrees for defaults

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

 



> From: bibendi <bibendi@xxxxx>
>
> Look in the config file .gittrees for a default repository and
> refspec or commit when they are not provided on the command line.
>
> Uses the .gittrees config file in a similar way to how git-submodule
> uses the .gitmodules file.

What the patch does can be read from the code, but what benefit
would users get by the extra file?

>  cmd_pull()
>  {
> -	ensure_clean
> -	git fetch "$@" || exit $?
> -	revs=FETCH_HEAD
> -	set -- $revs
> -	cmd_merge "$@"
> +    if [ $# -ne 1 ]; then

Broken indentation?

> +	    die "You must provide <branch>"
> +	fi

It used to allow "git fetch $there" and let the configured
remote.$there.fetch refspec to decide what gets fetched, and also it
used to allow "git fetch $there $that_branch" to explicitly fetch
the named branch.  But this change insists that the user has to give
what gets fetched from the command line and forbids the user from
giving where to fetch from, it seems.  Isn't it a regression?  Why
is it a good idea to forbid such uses that the script used to
accept?

The proposed log message does not explain why it is not a
regression, or why accepting some use patterns that the script used
to allow was a bug that needs to be diagnosed with this new
conditional.

> +	if [ -e "$dir" ]; then
> +	    ensure_clean
> +	    repository=$(git config -f .gittrees subtree.$prefix.url)
> +	    refspec=$1
> +	    git fetch $repository $refspec || exit $?
> +	    echo "git fetch using: " $repository $refspec

Why are these variable references outside the dq pair?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[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]