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

[Paul Campbell <pcampbell@kemitix.net>]

On Mon, Mar 11, 2013 at 3:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> From: bibendi <bibendi@bk.ru>
>>
>> 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?

How about:

"Usually push and pull are to the same repository/branch that they were
originally added from. Add stores the repository/branch in .gittrees
which push and pull can now default to if not provided on the command
line."

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

I think the patch was based on an older version of git-subtree before
"git fetch $there" support was added. Will need to update it.

>> +     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?
>

They're inside now.

Rerolling once I figure out the update for "git fetch $there" support.

-- 
Paul [W] Campbell