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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:21

From: bibendi <redacted>

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?

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

From: Paul Campbell <hidden>
Date: 2016-06-15 22:56:30

On Mon, Mar 11, 2013 at 3:35 AM, Junio C Hamano [off-list ref] wrote:
quoted
From: bibendi <redacted>

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."
quoted
 cmd_pull()
 {
-     ensure_clean
-     git fetch "$@" || exit $?
-     revs=FETCH_HEAD
-     set -- $revs
-     cmd_merge "$@"
+    if [ $# -ne 1 ]; then
Broken indentation?
quoted
+         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.
quoted
+     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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help