Re: [PATCH] push: Provide situational hints for non-fast-forward errors

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Mar 14, 2012, 09:06 UTC
Message-ID: <vpqipi7zh3n.fsf@bauges.imag.fr>
In-Reply-To: <20120313232256.GA49626@democracyinaction.org>

Christopher Tiwald <christiwald@gmail.com> writes:

Show 6 quoted lines

> 2) If you push to a shared repository others push to, and your local
> tracking branches are not kept up to date, the 'matching refs' default
> will generate non-fast-forward errors on outdated branches. If this is
> your workflow, the 'matching refs' default is not for you. Consider
> setting the 'push.default' configuration variable to 'upstream' to
> ensure only your checked-out branch is pushed.

Very good point.

Depending on the outcome of the discussion in the thread about 'push.default', you may want to suggest 'current' instead of upstream: http://thread.gmane.org/gmane.comp.version-control.git/192547/focus=192694

Actually, if the user has 'push.default=matching', the least surprising move from this value is 'push.default=current', that will push a subset of what used to be pushed, and won't change the target branch.

Show 6 quoted lines

> +static const char *message_advice_pull_before_push[] = {
> +	"To prevent you from losing history, non-fast-forward updates to HEAD",
> +	"were rejected. Merge the remote changes (e.g. 'git pull') before",
> +	"pushing again. See the 'Note about fast-forwards' section of",
> +	"'git push --help' for details."
> +};

Your patch removes the _(...) around the string, which breaks the internationalization.

Show 7 quoted lines

> +static const char *message_advice_use_upstream[] = {
> +	"By default, git pushes all branches that have a matching counterpart",
> +	"on the remote. In this case, some of your local branches were stale",
> +	"with respect to their remote counterparts. If you did not intend to",
> +	"push these branches, you may want to set the 'push.default'",
> +	"configuration variable to 'upstream' to push only the current branch."
> +};

I'd give the full cut-and-paste ready command to set the variable, to help the user who doesn't know what "configuration variable" really means in the context of Git:

... you may want to run

  git config push.default upstream (or current, if you like my remark above)

to ask Git to push only the current branch from now on.

(I'd advise "git config" without "--global" here, because the user may want to do something else in other repositories)

> +	for (i = 0; i < ARRAY_SIZE(message_advice_pull_before_push); i++)
> +		advise(message_advice_pull_before_push[i]);

I'm no expert in gettext, but I think the internationalization people will have a hard time dealing with a single message split accross an array.

Actually, I prefer the effect of a single advise() call (i.e. say "hint:" just once, not for each line), but this part is subjective.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/