Re: [PATCH 1/2] commit: do not cleanup template

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

 



These two commits change the longstanding behaviour. It is hard to tell if
the existing behaviour is something people who use commit log message
templates have relied on (in which case your patch will be a regression to
them) or they all shared the sense of annoyance and have been wishing to
be improved (in which case nobody will be inconvenienced with your patch).
They need to be explained a bit better why these changes are needed.

My gut feeling is that it is relatively easy to justify [1/2]. We prepare
the message file to be edited by the committer in sb by reading from
different message sources, and usually we would want to clean up what is
in sb before writing it out, but you may want to enforce a particular
format by using the template (such as having a leading whitespace) and the
way to tweak and fix that appearance should be by editing the template,
not by automatically running stripspace() here [*1*] when the message came
from the template.

I cannot tell offhand how you justify [2/2], though. What motivated you to
remove this blank line? At this point in the codepath, it does not look
like it should matter if the original message came from your template or
from somewhere else.

If the blank line is unneeded after "# You may be committing a merge..."
for readability, wouldn't that blank line be unneeded when you took the
message from other places, no?

It might make sense to move that newline before "if (whence != FROM_COMMIT)"
block, though, to make the logic easier to follow, regardless of the use
of the template file.

[Footnote]

*1* I however have a feeling that this call is here because other people
wanted to clean it up here because otherwise mistakes like trailing
whitespaces or excess blank lines and whatnot made in the template will
propagate down to the actual commit log message. But we run stripspace
again after the editor returns, so I think that worry is unfounded if we
are using an editor.
--
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]