Christopher Tiwald <christiwald@xxxxxxxxx> writes:
> One quick, slightly-off-topic question: I'd like to take another crack
> at the patch's commit message, to implement some of your language
> suggestions and clean it up further. Is it reasonable for me to wait a
> few days for additional comments or updates, squash together these
> fixups into a single v2 patch (assuming one patch is a logical unit for
> it), then resubmit?
Surely.
After reading the fix-up patch again, I actually have a couple of
comments/reservations myself.
(1) I suggested (and the fix-up patch does so) to use a single existing
advice configuration, but if you read the description in my response
to Clemens carefully, you may realize that at least the configuration
for "Here is how to deal with your current branch" and "Here is how
for the rest of your branch" might be better if they are separate
variables. The user may fix current branch (i.e. "pull then push"),
set the advice.pushNonFastForward to false thinking that he learned
everything there to know about non fast-forward, and then get another
failure from "git push" because other branches are still behind, but
with my "fix-up" patch, we would no longer give advice to him.
(2) The advice to "Your current branch is OK but you are also pushing
others that do not fast-forward" only talks about "check out, pull
and then push", but an equally plausible solution may be "don't push
other branches---you are not working on them right now". Both of our
versions have this issue.
I didn't trace the logic flow, though. If this advice is issued only
to the user who explicitly said he wants to push these other branches
(e.g. has "push.default = matching" in the config and gave no command
line options, or gave refspec on the command line to tell us to push
these other branches), then the wording is OK.
But for the purpose of helping people who may be surprised by the
current "matching" default, I think we should detect this very narrow
case:
- The user did not give us any refspec from the command line; and
- The user does not have push.default set to matching (either the
user does not have any push.default, or it is set to something
else); and
- The remote.$name.push would not push the branch other than the
current branch.
When these three conditions hold, we can be sure that the user worked
on more than one branch and did "git push $there" without telling us
what to push, and we defaulted to push "matching" and failed on stale
branches that the user hasn't been working on. In that case, "don't
push other branches---perhaps push.default needs to be set" may be a
far more appropriate advice.
So, the third case may have to be split further into two.
--
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