Re: [PATCH] Allow selection of different cleanup modes for commit messages
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:01
Linus Torvalds [off-list ref] writes:
On Sat, 22 Dec 2007, Alex Riesen wrote:quoted
+ if (!no_edit) {This is unrelated to the rest of the patch, but I do think double negations are horrible, so I thoink we should probably make the "no_edit" flag change meaning, and call it "run_editor" or something. ... So I wonder if we should perhaps: (a) Only add these messages at all when we do *not* do CLEANUP_ALL ... (b) Add a a new line to replace he "will not be included" message, ie ... I personally would prefer (a) - since anybody who then explicitly uses --cleanup={space|none} would presumably already know what he is doing. But this is not a huge deal. Regardless, the patch looks ok, so you can add a "Acked-by:" from me.
I was composing essentially the same message, except my preference was (as you can guess by the fact that I hinted the additional instruction) (b), but I agree that (a) is better at least for now because the user has to ask for verbatim every time (i.e. there is no config). By the way, the "if (!no_edit)" conditional itself you quoted above as the first thing in your message is not needed at all. If no_edit is set, the function already has returned and we would not reach that point. So a third round?