plone / mr.roboto

Keep github and jenkins in sync
http://jenkins.plone.org/roboto
1 stars 3 forks source link

github user with developer access is no longer recognised as contributor by mr.roboto #163

Closed fredvd closed 1 month ago

fredvd commented 1 month ago

The following issue surfaced at the Salamina-Sprint. User @folix-01 gets a mr.roboto message on his pull request on plone/volto that his e-mail address is no longer recognised.

"""Roman Kysil your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request. """

https://github.com/plone/volto/pull/6332

@ericof checked the mr.roboto code and we noticed some changes last week made by @davisagli and @stevepiercy . But we don't directly see how they could impact this.

I checked with Roman:

Roman is plone org member, member of the Developers Team. The only difference I see with him and @giuliaghisini is that she has Maintain Role on @plone/volto because she is a Volto-Team member.

fredvd commented 1 month ago

Erico drilled down to the following line in subscribers.py where the contributor check possibly fails, but we don't see the connetion yet with recent changes or wrong settings.

Maybe to avoid/work around this issue: we could move this check to github itself and check if a user is in the Contributors (or Development )Teams: the foundation contributor process is that the secrerary after a sent in Contributor Agreements adds them to the Contributors team.

Would require new workflows, checks and other re-configurations....

stevepiercy commented 1 month ago

Our changes were only to the text of the messages, not the validation of an email address. Additionally the text of the nag message on that PR has not changed from before our merge, so I assume the changes have not been released or deployed.

This must be an account configuration issue.

Where do we compare the email from the PCA against those in GitHub?

fredvd commented 1 month ago

@stevepiercy Yes, that was what also concluded. I chatted with Maurits, we've found a possible cause, in the branch/PR if you look at the commit message he was mixing e-mail addresses and likely the first address on the first commit is picked up and considered.

But this is also where Github can insert an anonymized @github.com address if you enable that option in your settings.

I pointed @folix-01 to the issue with his e-mail addresses, he is going to clean up the branch with a squash or similar and re-check.

This is very likely the issue, so I'll close it. Otherwise we can still re-open.

fredvd commented 1 month ago

Ah: I don't want to investigate this too much at the moment while in Ferrara, but I'd like to test it a bit more and see if we can add this to the developer documentation as a warning not to anonymize your e-mail address.

stevepiercy commented 1 month ago

Link to GitHub documentation.

https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-personal-account-on-github/managing-email-preferences/setting-your-commit-email-address

There may be a valid reason to anonymize one's email address in commits, such as avoiding harassment. Do we want to support that? @fredvd @mauritsvanrees

If not, or until then, we should add a note to contributing documentation and the PCA page on plone.org. I'll create an issue in both repos stating that "All commits must use the email address on the PCA and be registered in the contributor's GitHub account."

Also from what I can figure out from the code, the comparison of email addresses is between the emails on GitHub and the commits, not what I mistakenly assumed was some stored list of PCA emails.

stevepiercy commented 1 month ago

I'm reopening because I think we need to update the text of the auto-nag regarding commit emails in https://github.com/plone/mr.roboto/pull/162/files#diff-121d1f42b03c48c9e5c32ef2d984bf6bbc3b06bf25cdef699a652588ff50535fR277

stevepiercy commented 1 month ago

Issue created:

https://github.com/plone/documentation/issues/1716

stevepiercy commented 1 month ago

PRs made: