From: Eric Sunshine Date: Wed, 24 Jan 2018 18:39:39 GMT Subject: Re: [PATCH v3 2/3] t7505: Add tests for cherry-pick and rebase -i/-p Message-ID: In-Reply-To: <20180124123422.10637-3-phillip.wood@talktalk.net> On Wed, Jan 24, 2018 at 7:34 AM, Phillip Wood wrote: > Check that cherry-pick and rebase call the 'prepare-commit-msg' hook > correctly. The expected values for the hook arguments are taken to > match the current master branch. I think there is scope for improving > the arguments passed so they make a bit more sense - for instance > cherry-pick currently passes different arguments depending on whether > the commit message is being edited. Also the arguments for rebase > could be improved. Commit 7c4188360ac ("rebase -i: proper > prepare-commit-msg hook argument when squashing", 2008-10-3) apparently > changed things so that when squashing rebase would pass 'squash' as > the argument to the hook but that has been lost. > > I think that it would make more sense to pass 'message' for revert and > cherry-pick -x/-s (i.e. cases where there is a new message or the > current message in modified by the command), 'squash' when squashing > with a new message and 'commit HEAD/CHERRY_PICK_HEAD' > otherwise (picking and squashing without a new message). > > Signed-off-by: Phillip Wood > Reviewed-by: Eric Sunshine > Reviewed-by: Junio C Hamano Let's drop the Reviewed-by: from me. Although I spotted a minor portability issue while scanning a previous iteration, I did not read the patch closely enough to draw any conclusion of its overall correctness. Normally, a Reviewed-by: is given explicitly by a reviewer when confident that the patch is correct and meets the stated goals. I suspect that Reviewed-by: Junio ought, similarly, to be dropped.