Update sign-off location instructions #2096
Conversation
As per discussion in [1] the sign-off tag must be in the footer separated by a blank line from the commit body or header. Update the documentation example to abide by this rule and add a sentence making it explicit in the instructions. [1] #1636 (comment) Signed-off-by: Filip Jeremic <fjeremic@ca.ibm.com>
| manually adding the following line to the footer of the commit. | ||
| manually adding the following line to the footer of the commit. The footer must | ||
| be separated from the preceeding body or header with a blank line, and there must | ||
| not be a blank line following the footer. |
There was a problem hiding this comment.
and there must not be a blank line following the footer. Isn't this covered by the text a few lines down?
Remember, if a blank line is found anywhere after the
Signed-off-byline, the
Signed-off-by:will be considered outside of the footer, and will fail the
automated Signed-off-by validation
There was a problem hiding this comment.
Yes, just reinforces the previous sentence. I wanted to keep it concise and simple. I'm actually in favor of removing the "Remember, if a blank..." paragraph as it's to verbose. Thoughts?
| @@ -124,6 +126,7 @@ The guidelines are changed to: | |||
| message. | |||
|
|
|||
| Closes: #124 | |||
|
|
|||
There was a problem hiding this comment.
I don't believe this is necessary - both Closes and Signed off can be part of the footer.
There was a problem hiding this comment.
#1636 (comment) seems to imply otherwise. @IBMJimmyk can you validate the correctness of your comment?
There was a problem hiding this comment.
Actually @DanHeidinga there is a clear example of the check failing in #2098. You can see the failure output (https://ci.eclipse.org/openj9/job/PullRequest-SignedOffByCheck-OpenJDK10/244/):
13:32:51 ###################################
[Pipeline] echo
13:32:55 FAILURE - The following commits appear to have an incorrect sign-off
[Pipeline] echo
13:33:02 commit afc2c20e928648a6277b2d17f5eebd01b2e75699
13:33:02 Author: Daniel Hong <daniel.hong@live.com>
13:33:02 Date: Thu May 31 09:55:11 2018 -0400
13:33:02
13:33:02 Re-enable toLowerCase/toUpperCase acceleration
13:33:02
13:33:02 Modify API call so that caseConversionHelper recieves
13:33:02 underlying arrays and length of string.
13:33:02
13:33:02 Closes: #1934
13:33:02 Signed-off-by: Daniel Hong <daniel.hong@live.com>
[Pipeline] echo
13:33:05 ###################################
So we do in fact need a new line between the "Closes" and "Signed-off-by" tags in the footer.
There was a problem hiding this comment.
Note the PR referenced has since been fixed up but I've linked the failing job in the above comments. It's easily reproducible as well.
There was a problem hiding this comment.
That just means the check we are implementing hasn't been implemented correctly. Its not the Eclipse ip-validation which was failing.
There was a problem hiding this comment.
So do we need an issue against our own check to handle this case and close this PR?
There was a problem hiding this comment.
There already is an issue open, the check is WIP and being testing on the OpenJ9 repo.
ibmruntimes/openj9-openjdk-jdk9#58 (comment)
#1941
Yes I think we can close this PR.
|
Closing as per #2096 (comment). |
As per discussion in [1] the sign-off tag must be in the footer separated by a blank line from the commit body or header. Update the documentation example to abide by this rule and add a sentence making it explicit in the instructions.
[ci-skip]
[1] #1636 (comment)
Signed-off-by: Filip Jeremic fjeremic@ca.ibm.com