Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update CONTRIBUTING.md with information about permissions #3590

Open
aleesteele opened this issue Apr 2, 2024 · 3 comments
Open

Update CONTRIBUTING.md with information about permissions #3590

aleesteele opened this issue Apr 2, 2024 · 3 comments

Comments

@aleesteele
Copy link
Member

aleesteele commented Apr 2, 2024

Summary

During our March 2024 Onboarding call, @CeilidhWelsh and @Susana465 brought up an interesting discussion about permissions: can newcomers merge pull requests? Who has admin rights within the project, and how do you get those permissions? How can we make sure to add newcomers to the repository in the onboarding calls so that they can feel empowered to contribute through closing PRs (or otherwise)? What about 'good-first-pull-requests' - such as adding/closing Contributor Bot? Are these open for folks who have otherwise added in their info to the repo through other means? Should these Onboarding Calls explicitly be places to give permissions to the repository - so that folks can do these kinds of reviews?

During the onboarding call, we had a newcomer to the project that was asking about how they could help to review small Issues and PRs as a beginner. They found an open pull request with some notes that had been reviewed but not merged, and after adding them to the repo, we had a live merge during the call which was great! 🎉 This instance created a number of questions about whether there are actually "good first PRs", when folks don't have merge access from the get-go, in a large repository like TTW.

What needs to be done?

  • Add information to CONTRIBUTING.md file about permissions for editing the repository

Who can help?

  • A community member who is familiar with how we use permissions within the community, and how to document them openly
  • Any community member who is undersure of how permissions work in tThe Turing Way community, and can help make sure our documentation is readable for them.

Updates

@aleesteele
Copy link
Member Author

aleesteele commented Apr 2, 2024

Adding these notes from @da5nsy

No specific permissions are required to create an issue or PR. The first time we generally see a need for permissions is when merging a PR, and that's only if the new contributor is the one actually pushing the merge button.
Long-term, it does make sense for us to develop a clear plan about permissions, but I don't think it's a blocker in this situation. Does that make sense?

And from @malvikasharan

The repo is open for anyone to create PR. The only difference 'write access' creates is that contributors can create branch instead of forking the repo. But general purpose has been to allow anyone who would like to merge to the repo - for example when working on a chapter.

@Susana465
Copy link
Collaborator

Susana465 commented Apr 2, 2024

Hi, adding some extra info on this. Which I think is important in terms of practical terms of who runs events such as collaboration cafes and/or onboarding calls.

As Anne pointed out: "They [the newcomer] found an open pull request with some notes that had been reviewed but not merged, and after adding them to the repo, we had a live merge during the call which was great! 🎉 "

What is important here is that if Anne, who has admin rights in GitHub, had not been there, neither Ceilidh or me could have added this new user to the GitHub repo for them to be able to merge this PR. So this means:

1- Not anyone can close PR merges ( as Danny mentions:

No specific permissions are required to create an issue or PR. The first time we generally see a need for permissions is when merging a PR, and that's only if the new contributor is the one actually pushing the merge button. )

1.1. - If Anne had not been there to add this new user, one of us would have done the merge, but not the newcomer herself. This is not necessarily a bad thing, and perhaps keeping things this way acts as some form of safeguarding (why is it this way, though? the answer to this question is what I can't find), but it also means newcomers have somewhat restricted access to github until added to the TTW repo as a contributor.

2 - Ceilidh and I don't have admin rights, a key question therefore is: who gets to add permissions in TTW GitHub, and why? How are these rights decided?

@malvikasharan
Copy link
Collaborator

malvikasharan commented Apr 3, 2024

  1. We share maintainers and manager-level access with folks engaged in the core community such as WG and community support, such as you and Ceilidh (I have double-checked - I think Anne has given you the access after this onboarding call situation?). These access levels can create branches, and review and merge PRs - similar to anyone who has 'write' access (the access level we can give to any contributors).
    I don't see any situation under which a newcomer needs to be given write access urgently (unless we are conducting GitHub training and want to show the difference between forking and creating a branch when writing a chapter or contributing) - meaning if someone requests to have write access, they will likely have to wait, but that shouldn't block their work.

  2. Admin rights are reserved for the project team and infrastructure leads as a person with admin rights can delete anything (intentionally or by error) and we don't want to have that risk. This is a general rule of thumb across open source projects and we want to follow that to ensure responsibility for infrastructure maintenance.

I hope I am capturing these states from the project correctly, but keen to hear what potential issue we can have with this framework - and if there is better model we can identify that doesn't risk the project's infra maintenance work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants