Re: [PATCH v3 1/4] Introduce CHERRY_PICK_HEAD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jay Soffian <jaysoffian@xxxxxxxxx> writes:

> diff --git a/builtin/merge.c b/builtin/merge.c
> index 9403747..454dad2 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -971,6 +971,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		else
>  			die("You have not concluded your merge (MERGE_HEAD exists).");
>  	}
> +	if (file_exists(git_path("CHERRY_PICK_HEAD"))) {
> +		if (advice_resolve_conflict)
> +			die("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).\n"
> +			    "Please, commit your changes before you can merge.");
> +		else
> +			die("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).");
> +	}

Micronit: "Please, commit your changes before you can merge."

 - We are not merging in this codepath to begin with;

 - I'd suggest rephrasing this (together with "MERGE_HEAD" codepath) to
   something like:

	"Commit your changes first before retrying."

> +test_expect_success 'cherry-pick --no-commit sets CHERRY_PICK_HEAD' '
> +
> +	git checkout -f initial^0 &&
> +	git read-tree -u --reset HEAD &&
> +	git clean -d -f -f -q -x &&
> +
> +	git update-index --refresh &&
> +	git diff-index --exit-code HEAD &&

Getting tired of seeing these five lines repeated over and over.  Perhaps
it is time to introduce:

	pristine_detach () {
        	git checkout -f "$1^0" &&
                git read-tree -u --reset HEAD &&
                git clean -d -f -f -q -x
	}

I don't think "diff-index --exit-code HEAD" is necessary as the point of
this testsuite is not about testing "read-tree --reset", and the index
refresh is just a prerequisite for diff-index that can go together with
it.

> +	git cherry-pick --no-commit base &&
> +
> +	test_cmp_rev base CHERRY_PICK_HEAD

If the next "git commit" would notice and use this information, that would
introduce an unpleasant regression to one use case in my workflow, which
is to pick and consolidate one or more small pieces made on a private
"misc" branch, possibly with a bit of further work, into a new commit with
a readable explanation that is unrelated to any of the original commits:

	git cherry-pick --no-commit $some_commit
	git cherry-pick --no-commit $another_commit ;# optional
	edit ;# optional
        git commit

I'd prefer to see a way to tell cherry-pick not to leave CHERRY_PICK_HEAD
behind when "cherry-pick --no-commit" results in a successful cherry-pick
to avoid a backward incompatibility surprise.  Otherwise people need to
retrain their fingers to say --reset-author when they run "git commit".

Other then the above three points, this patch looks good.  Thanks.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]