Hi everyone, I am currently working on FINERACT-2462, which has me adding a github action that checks whether there is One Commit Per User on a PR. I am trying to handle the edge case where the git email is not the same as one of the emails registered on Github. In this case, we cannot access the unique github cli author.id field, which I was planning to use to verify One Commit Per User. My first idea was to default to git email or git username when this occurs. However, this can also lead to issues, since multiple users might leave their git username and emails blank or with some placeholder value.
To handle both of these issues, my idea was to ensure that anyone committing in a PR must have committed with their git email properly configured to be the same as one of their github emails. Would this be okay to enforce in the scope of our project? Thank you all.
Make sure to not make this strict check. On every push and reopen. It must comment to merge requester to squash commit.
🧐 should be one commit per PR, not one commit per user
@Aman-Mittal Sounds good! I will include synchronize as well to account for erroneous PR changes. Just to clarify, the way I've currently implemented produces an error in the checks on failure, which can be clicked for the error message. Did you want an actual PR comment as well? Either way works just wanted your preference!
@Adam Monsen (meonkeys) That certainly makes it easier!
Hi! That sounds like the right approach. Relying on raw git usernames/emails can definitely be flaky due to duplicates or typos. Enforcing that the commit email matches a registered GitHub email is a standard practice in many projects to ensure proper attribution and CLA checks. I think it makes sense to move forward with that requirement.
@Ismael Sallami Sounds good! I'll go ahead with this approach and also add a check for this in my PR. Thank you!
@Ismael Sallami wrote:
> Enforcing that the commit email matches a registered GitHub email is a standard practice in many projects to ensure proper attribution and CLA checks
Is it now? Can you link to some evidence of this please?
@Edward K thanks for your comms in here and on the mailing list. This discussion is excellent. Have you opened a PR yet? No worries if not, I'm just curious if there's something I can look at yet
Hi @Adam Monsen (meonkeys)! Thank you for the kind words! I haven't opened a PR yet, as I've been testing the code on my personal fork with a few test cases. But it will be up shortly, and I'll be sure to tag you!
👍 1
Hi @Adam Monsen (meonkeys),
You are right to ask. I was referring specifically to projects that enforce CLA (Contributor License Agreement) or DCO checks via CI pipelines. In those contexts, having a commit email that matches the signed-off identity/account is usually a requirement for the checks to pass.
I may have generalized by calling it a universal 'standard practice', but that was the specific use case I had in mind regarding strict attribution.