| Message ID | 20200319132957.17813-2-harshitjain1371999@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t4131: update test script | expand |
Hello Harshit,
> 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.
Maybe add a commit title and then have a body? To do so, do a 'git commit' instead of 'git commit -m "message"'. This will open a text editor
in which you can edit your commit message. You may refer to this answer I gave on StackOverflow on commit messages:
https://stackoverflow.com/a/60755299/10751129
Also, commit messages are generally around 72 characters per line. What are the
style violations you are talking about BTW?
The commit title can be of the form:
t4131: modernise style
<<commit description>>
Regards,
Shourya Shukla
Hi Shourya, > > 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. > > Maybe add a commit title and then have a body? To do so, do a 'git commit' instead of 'git commit -m "message"'. This will open a text editor > in which you can edit your commit message. You may refer to this answer I gave on StackOverflow on commit messages: > > https://stackoverflow.com/a/60755299/10751129 I used 'git commit' only and not 'git commit -m "message". But apparently, the git format-patch tool takes the first line of commit message i.e. the commit title as the file name and the lines after that as the text for the body. And hence, the patch emails, just start with the commit description and not the commit title. So, should I explicitly add the commit title in the patch files generated or else how to handle this? > 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. Regards, Harshit Jain
Harshit Jain <harshitjain1371999@gmail.com> writes: >> > 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. >> 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.
diff --git a/t/t4131-apply-fake-ancestor.sh b/t/t4131-apply-fake-ancestor.sh index b1361ce546..828d1a355b 100755 --- a/t/t4131-apply-fake-ancestor.sh +++ b/t/t4131-apply-fake-ancestor.sh @@ -17,8 +17,8 @@ test_expect_success 'setup' ' test_expect_success 'apply --build-fake-ancestor' ' git checkout 2 && - echo "A" > 1.t && - git diff > 1.patch && + echo "A" >1.t && + git diff >1.patch && git reset --hard && git checkout 1 && git apply --build-fake-ancestor 1.ancestor 1.patch @@ -26,8 +26,8 @@ test_expect_success 'apply --build-fake-ancestor' ' test_expect_success 'apply --build-fake-ancestor in a subdirectory' ' git checkout 3 && - echo "C" > sub/3.t && - git diff > 3.patch && + echo "C" >sub/3.t && + git diff >3.patch && git reset --hard && git checkout 4 && (
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. Signed-off-by: Harshit Jain <harshitjain1371999@gmail.com> --- t/t4131-apply-fake-ancestor.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)