Re: [GSoC][PATCH 1/2] t4131: modernize style

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 19, 2020, 21:55 UTC
Message-ID: <xmqq5zf0dn7n.fsf@gitster.c.googlers.com>
In-Reply-To: <CACuU+s9WbKyg3P+ai8pnQqvRJDv+OoyqU-730K=1R7uasc=09g@mail.gmail.com>

Harshit Jain <harshitjain1371999@gmail.com> writes:

Show 5 quoted lines

>> > The tests in 't4131-apply-fake-ancestor.sh' were written a long
>> > time ago, and have a few style violations. Update it to adhere
>> > to the CodingGuidelines.
>> ...
> I used 'git commit' only and not 'git commit -m "message". But

I'd suggest developers, especially the new ones, to stay away from using '-m "message"' form, too.

In your editor, you would probably have written something like

	-- -- -- -- -- the contents of editor window -- -- -- -- --
	t4131: modernize style

	The tests in 't4131-apply-fake-ancestor.sh' were written ...
	...

	-- -- -- -- -- the contents of editor window -- -- -- -- --

As you observed, the first paragraph of the log message text is taken as the title of the commit, and "git format-patch" places the title on the "Subject:" line (if you had more than one line in the first paragraph, since the payload on the "Subject: " line has to be a logically single line, you'd end up getting a single long line that has the contents on all lines in the first paragraph).

The second and subsequent paragraphs become the body of the message.

Your title looks reasonable; there is nothing that needs to be "fixed" or "improved" there.

Your second paragraph is not so good---it should wrap the lines at a reasonable length (say 65-70 columns).

Your last paragraph, which consists of a single "Signed-off-by:" line in this case, is good. It matches the identity recorded on the "From:" line of the message.

Show 5 quoted lines

>> Also, commit messages are generally around 72 characters per line. What are the
>> style violations you are talking about BTW?
>
> The git coding guidelines says that we shouldn't have a space after
> the redirection operators, hence I corrected this in the test file.

That is a good thing to write in the commit log message.

"written a long time ago" does not have much value by itself (it does serve as a backstory to explain a half of why it does not use the more modern style, though). "have a few style violations." is almost meaningless (otherwise, you would not be doing a "modernize style" patch in the first place ;-).

	t4131: modernize style.

	The tests in t4131 leaves a SP between a redirection
	operator and the file that is the redirection target,
	which does not conform to the modern coding style.

	Fix them.

	Signed-off-by: ...

perhaps.