Christopher Tiwald <christiwald@xxxxxxxxx> writes:
> @@ -177,6 +192,16 @@ static int push_with_options(struct transport *transport, int flags)
> {
> int err;
> int nonfastforward;
> + struct branch *branch;
> + struct strbuf buf = STRBUF_INIT;
> +
> + branch = branch_get(NULL);
> +
> + if (branch) {
> + strbuf_addstr(&buf, transport->remote->name);
> + strbuf_addstr(&buf, "/");
> + strbuf_addstr(&buf, branch->name);
> + }
The "buf" is a horrible name for a variable that is used to hold states
in a long haul that has to span multiple hunks in a patch. Please name
it after what the value means.
> @@ -201,7 +226,18 @@ static int push_with_options(struct transport *transport, int flags)
> default:
> break;
> case NON_FF_HEAD:
> - advise_pull_before_push();
> + /* Branches configured for octopus merges should advise
> + * just 'git pull' */
> + if (branch->remote_name &&
> + branch->merge &&
> + branch->merge_nr == 1 &&
> + !strcmp(transport->remote->name, branch->remote_name) &&
> + !strcmp(strbuf_detach(&buf, NULL),
> + prettify_refname(branch->merge[0]->dst))) {
Why detach? buf_to_be_renamed_more_sanely.buf, perhaps?
Is comparison between whatever buf has and the result of prettify safe
and sane? After all, prettify is a random abbreviation that is meant
for human consumption, assuming that the reader is intelligent enough to
guess "Ah, it must be a tag" when she sees "v1.0" which is the result of
stripping the leading "refs/tags/" out.
This part should be using straight strcmp against branch->merge[0]->dst,
which means buf_to_be_renamed_more_sanely in the earlier hunk needs to
be computing what's tracked and merged correctly using the configured
refspec mapping, perhaps?
--
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