Skip to content

Conversation

@AaronBallman
Copy link
Collaborator

See https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it/74223 for details about why this is important to the community.

Note, we currently have soft enforcement for this requirement in the form of a bot which posts comments letting patch authors know their email is private, so we're already setting expectations in practice; this PR is documenting those expectations for clarity.

See https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it/74223
for details about why this is useful.

Note, we currently have soft enforcement for this requirement in the
form of a bot which posts comments letting patch authors know their
email is private.
@AaronBallman
Copy link
Collaborator Author

Another reason why this is potentially becoming more important in the relatively near future is because the proposed governance process intends to use GitHub APIs to verify contributors for voter eligibility and as a way to send ballots/voting instructions to folks. That's not what's driving this PR, but it is what got me motivated to post it.

@joker-eph
Copy link
Collaborator

Are we gonna "upgrade" the bot message from a soft message to more of a requirement?
Considering the current dynamic since I raised this, I just ignore the bot and merge PRs (like most people it seems) at the moment.

@AaronBallman
Copy link
Collaborator Author

Are we gonna "upgrade" the bot message from a soft message to more of a requirement? Considering the current dynamic since I raised this, I just ignore the bot and merge PRs (like most people it seems) at the moment.

To be honest, I don't know. I figured we'd get the policy down on paper first, then we can post another RFC to see if we want to strengthen the enforcement of the policy further. Personally, I'm comfortable with the soft action so long as the person landing changes on behalf of someone is willing to take responsibility for reverts if necessary. But the RFC thread was mixed on how firm we should be with enforcement, so I didn't want to change behavior with this PR.

@asl
Copy link
Collaborator

asl commented Sep 27, 2024

We can certainly upgrade the bot message and make it a required check. However, this would cover only maybe half of the issue. The bot checks for email addresses used in commits. However, the email address is rewritten during "Squash & Merge" and it seems we neither cannot control nor prevent this.

Looks like the only solution is to disable direct merges at all? And delegate to some bot?

@Bryce-MW
Copy link
Member

Bryce-MW commented Oct 1, 2024

I had a similar issue at work where we wanted a check on the final commit created by "Squash & Merge" and not just the earlier commits. We didn't end up going with it but the solution I came up with was using GitHub merge queues and having just that one check enabled

@AaronBallman
Copy link
Collaborator Author

Ping on the documentation changes.

@dwblaikie
Copy link
Collaborator

Just a +1 for the general direction. The CoC Committee has encountered this problem recently too - no ability to privately communicate with a community member/contributor if they don't have a public email address associated with their github account.

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this policy change is needed and great.

@AaronBallman
Copy link
Collaborator Author

I've updated based on the feedback from @rnk

Copy link
Collaborator

@rnk rnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the edit!

@AaronBallman AaronBallman merged commit bf1a554 into llvm:main Oct 17, 2024
9 checks passed
@AaronBallman AaronBallman deleted the aballman-public-emails branch October 17, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants