docs: expand on DCO using manual sign-off #744
Conversation
Signed-off-by: tarilabs <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ederign The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
bump @terrytangyuan @andreyvelich could this be merged, please? (I'm going through some backlog of pending PRs...) thanks |
| @@ -5,7 +5,7 @@ You can also configure it globally (for every repo on your machine) by copying t | |||
|
|
|||
| You can also sign off your contributions manually by doing ONE of the following: | |||
| * Use `git commit -s ...` with each commit to add the sign-off or | |||
| * Manually add a `Signed-off-by: Your Name <[email protected]>` to each commit message. | |||
| * Manually add a `Signed-off-by: Your Name <[email protected]>` to each commit message; please note that Name and Email of sign-off must match the commit's Author, in other words the `user.name` and `user.email` of the git configuration used when creating the commit. Using `git commit -s ...` does this automatically. | |||
There was a problem hiding this comment.
I am not sure if this is too verbose here for an alternative step. We should recommend the automatic way instead of the manual way.
There was a problem hiding this comment.
thank @terrytangyuan , the automatic way is listed just above this bulletpoint.
this is imho nice "gotchas" for those situation where you have to resolve something manually; it happen a couple of times to me, and in the first occurence I lost time digging for this information, so my hope is this will spare some time for others facing potentially same issues when those rare occurences of having to manually signing off
There was a problem hiding this comment.
Were there any error messages when the name/email mismatches? I wonder if we should defer this to the DCO check itself since I'd imagine there are many other gotchas. This one talks about the name but there can be a mismatch in primary email as well
There was a problem hiding this comment.
This one talks about the name but there can be a mismatch in primary email as well
this comment actually refers to the commit Author, as a whole, making reference to the Name and Email explicitly.
I wonder if we should defer this to the DCO check itself since I'd imagine there are many other gotchas.
I've added this comment after some little frustration because I didn't find this information easily, and my hope would have been to spare others of the same.
If we think however this is resulting in the doc instead being too verbose, we can close this PR.
There was a problem hiding this comment.
Ok if DCO check doesn't provide much information then this would be useful to have. Thanks for clarifying.
|
Thank you for this PR @tarilabs! |
The website currently already links back to this doc which has more details. |
But this doc has very little information. @terrytangyuan Is there any problem to just move it in Kubeflow website ? |
|
This file has 13 lines of instructions, which could take 2/3 of the page or so (and that section is at the top of the page on the website). If contributors are already familiar with DCO, they don't need the details. There are also instructions in this README that are specific to the commit hook so I think it would be better for code and instruction to stay close to each other. |
|
@terrytangyuan Why it will be the 2/3 page ? We can always create dedicated page in the website if we feel that this is additional information that users should read. I just think that having unify place for docs will be easier for us to maintain, and easier for users to search. Any thoughts @StefanoFioravanzo @hbelmiro ? |
I meant that if we move these lines to that page, it would take up 2/3 of my screen before I can read the next sub-section "Follow the code of conduct". That page is TLDR and especially intimidating for first contributors IMO. We should move details to other places. Just my two cents though. |
@hbelmiro wdyt?
I had this gotchas when DCO didn't pass despite I've used the hook (in a project, the Name was different).
I'm also not sure the email of commit "must" match the github primary email, but that's for another time.