RFC2: New criteria for commit access

I’m re-posting the final version of the proposal from the original RFC so it gets more visibility.

Requirements For Retaining Commit Access

In order to retain commit access, a user must have a total of 5 repository ‘activities’ in the last 12 months. These activities can be any one of the following:

  • Pushing a commit.
  • Merging a pull request (either their own or someone else’s).
  • Commenting on a PR.

Implementation

We will have automation that files a github issue every month which tags any users that don’t meet the threshold for retaining commit access. An example ticket can be seen here. If a user does not respond to the ticket within a month, their repository role will be changed from Write to Triage. This will remove their commit access but allow them to keep privileges to do other things like:

  • Apply/dismiss labels
  • Close, reopen, and assign all issues and pull requests
  • Apply milestones
  • Mark duplicate issues and pull requests
  • Request pull request reviews
  • Hide anyone’s comments

The issue will contain a friendly explanation of why we are doing this:

Regaining Commit Access

A user with the Triage role does not need any justification or have to meet any criteria to regain commit access. To restore their commit access, they just need to follow the process for outlined in the developer policy .

Other Changes

We will add automation to remind reviewers to merge a PR when they approve one from an author without commit access.

Impact

As of Feb 1, 2024 there were 1798 users with commit access to the llvm-project. If this proposal were to be adopted, 731 of those users would be switched from the write role to the triage role.

20 Likes

It seemed to me like there was sufficient consensus on the last proposal already. I also think we could likely go further than is proposed here.

But, as a first step this seems good.

Sounds good to me.

Sounds good to me. I wonder if there is an easy way to allow for authors without commit access to merge their PRs if they were approved. Would reduce the need for the automation in the “Other changes” and the want for the commit access.

Will only 731 users be affected or more community members are expected to be affected in the future? Current wording implies the former, as for my interpretation.

Do we need a justification for the policy change?

As far as I can tell, there is a support for the proposed policy change from the community members not affected by this change. Should we seek input from the people affected by the change?

Commit access review will become a monthly process. The 731 number comes from the first review, but each month there will be a new set of people who qualify for the triage role.

The justification is discussed in the other thread, but the main reason to do this is to protect against commit credentials being comprimised.

People can give input in this thread, but also each person will be asked if they want to keep their commit access, so they will have a chance to say ‘yes’ to that before any action is taken.

This sounds sensible to me, thank you!

Thank you so much for reposting this for increased visibility Tom!

Sounds reasonable to me.

Sounds good to me.

Regardless of the outcome of this RFC I’d love to see this!

1 Like

It is useful to call this out explicitly because you need to understand the entire proposal before knowing the actual impact is higher.

In the later versions of the RFC the justification wasn’t carried over. So it is unclear if it is still applicable or not.

Having “alternatives considered” section can be useful for the proposal. But given that nobody else is interested, I’m not pressuring you to add it. If you decide to wait until there is a higher demand, it’s up to you.

Opportunity to appeal the decision is a necessary but not a sufficient part of the policy change. Though I appreciate that the appeal process is simplified. But the availability of a recourse doesn’t justify a decision. Like the presence of Code of Conduct and moderators doesn’t justify the offensive behavior.

Overall, I find this treatment of infrequent contributors disgusting. Everybody in the community believes it is acceptable that infrequent contributors don’t deserve their opinion to be asked for. That it is OK to treat other people as a threat and a problem to be fixed. Apparently these are “community” norms and I won’t try to change its members because I believe into not forcing my will onto others.

Don’t see how that logically follows from this RFC at all.

2 Likes

Please show me where anybody else suggests to ask the infrequent contributors for their opinion on the policy that affects them directly.

How would you interpret the following passage?

It’s important that we can trust people with these kinds of privileges to ensure that our project’s software supply chain is secure. I think limiting commit access to only active contributors will significantly cut down on the risk of a bad actor obtaining commit access and compromising the project in some way.

To me it does sound like the infrequent contributors are considered a security threat.

If any of the facts are wrong, please don’t hesitate to point the specific mistakes out. It is totally possible I’ve missed something or made another mistake, so I value learning about such deficiencies.

The policy was posted publicly, where essentially anyone can comment. Are you suggesting that’s not sufficient? What would you propose to do instead?

It’s more that infrequent contributors are less likely to notice when someone has hijacked their credentials. It’s not really the individuals themselves who are a risk.

One could argue that such people could be frequent contributors elsewhere and in fact would notice a hijack. But I think someone who rarely commits to LLVM would not be seriously hampered by the need to get someone else to merge their PR.

2 Likes

I see your point. And I agree that the risk is in hijacking credentials and abusing permissions. But GitHub doesn’t really separate accounts and permissions. In my quick check of GitHub interface it’s not “somebody has granted you write permission to the repository” but “you are a member of committers team”. And in my experience (that can be wrong) people don’t separate accounts from themselves. For example, have I quoted “your words” or “words that credentials you own enabled you to post”? Again, I agree that technically the threat isn’t personal but it’s not unreasonable to perceive it as personal.

I agree that removing the commit access won’t really harm anyone (and I think I’ve never argued the opposite but that’s not my point). For myself it would be annoying and inconvenient, I would get a bruised ego but nothing bad would happen. And I guess that removing you from the committers won’t cause anything terrible. The harm I see is being done to the project itself. Alienating infrequent contributors isn’t likely to make them frequent contributors. And the lack of new contributors isn’t good for any OSS project’s health [long-term].

In the Developer Policy we have some examples when a wider dissemination of some information is required. And the process tend to involve people affected by a change. But I don’t think it is fruitful to continue arguing if a publicity of an action is sufficient or not.

Given that we discuss a specific solution and not a problem, I’m not sure there are good ways to address my concern. Ideally, the need to treat infrequent contributors better would shape the solution from the very beginning. For example, we can recognize that obtaining everyone’s opinion is challenging and putting the decision about the long-term project health into someone’s else hands is avoiding the responsibility. For example, we can check if infrequent LLVM contributors are indeed inactive accounts. Can check with GitHub their policies about inactive accounts and when accounts are marked as inactive. We can publish releases in a different way, not tying them to GitHub releases.

As for me, the biggest problem is that we discuss a solution with a binary yes/no answer and not a problem that can have other solutions which would be satisfactory to more people.

P.S. I’ll be on vacation, so my answers will be delayed.

There still appears to be consensus on this proposal, so I’m going to continue implementing this.

4 Likes

Thank you being thorough Tom!