LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 42428 - Enable GitHub Commit Access
Summary: Enable GitHub Commit Access
Status: NEW
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: unspecified
Hardware: PC Linux
: P enhancement
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks: github
  Show dependency tree
 
Reported: 2019-06-27 14:46 PDT by Tom Stellard
Modified: 2019-10-29 20:59 PDT (History)
10 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Stellard 2019-06-27 14:46:23 PDT
We need to grant GitHub commit access to all current SVN committers.
Comment 1 Jonas Hahnfeld 2019-09-21 01:27:07 PDT
This morning, I received an invitation to join llvm/llvm-project. Does that mean we will not be allowed to push to llvm-zorg and llvm-test-suite (by default)?
If that was not the intention, I think it would make sense to put all contributors into a team. That way you can manage permissions more globally and add other repos if needed.
Comment 2 Tom Stellard 2019-09-23 10:45:16 PDT
(In reply to Jonas Hahnfeld from comment #1)
> This morning, I received an invitation to join llvm/llvm-project. Does that
> mean we will not be allowed to push to llvm-zorg and llvm-test-suite (by
> default)?

I was trying to start everyone out with the minimum permissions and then expand as needed.  Do you have recommendations for other default repos besides llvm-zorg and llvm-test-suite?  

> If that was not the intention, I think it would make sense to put all
> contributors into a team. That way you can manage permissions more globally
> and add other repos if needed.

The reason I did not create a team is that you need to be an organization member to be added to a team and organization members have some extra permissions within the project that outside collaborators don't have.
Comment 3 Jonas Hahnfeld 2019-10-03 04:00:14 PDT
(In reply to Tom Stellard from comment #2)
> (In reply to Jonas Hahnfeld from comment #1)
> > This morning, I received an invitation to join llvm/llvm-project. Does that
> > mean we will not be allowed to push to llvm-zorg and llvm-test-suite (by
> > default)?
> 
> I was trying to start everyone out with the minimum permissions and then
> expand as needed.  Do you have recommendations for other default repos
> besides llvm-zorg and llvm-test-suite?

Not right now, but just in case there will be a new repo in the future, do you want to add all contributors manually / with a script? Just ticking a box for a whole team would be much easier.

> > If that was not the intention, I think it would make sense to put all
> > contributors into a team. That way you can manage permissions more globally
> > and add other repos if needed.
> 
> The reason I did not create a team is that you need to be an organization
> member to be added to a team and organization members have some extra
> permissions within the project that outside collaborators don't have.

I think you are referring to this https://help.github.com/en/articles/permission-levels-for-an-organization?

AFAICS members have the following permissions:
 * Create repositories (but can be restricted, see https://help.github.com/en/articles/restricting-repository-creation-in-your-organization)
 * Create teams (but can be restricted, see https://help.github.com/en/articles/setting-team-creation-permissions-in-your-organization)
 * Create project boards (although it's not clear to me after reading https://help.github.com/en/articles/project-board-permissions-for-an-organization; it appears you need "project board admin permissions"?)
 * Additional permissions to view a few things (all teams, insights)
 * Ability to hide comments, but that seems to apply to everyone who has write access to a repo (see https://help.github.com/en/articles/managing-disruptive-comments)

Is there anything else that "normal" contributors should not be allowed to do and that cannot be disabled?
Comment 4 Kristina Brooks 2019-10-10 21:28:41 PDT
(In reply to Jonas Hahnfeld from comment #3)
> 
> Not right now, but just in case there will be a new repo in the future, do
> you want to add all contributors manually / with a script? Just ticking a
> box for a whole team would be much easier.
>

That does seem like a better way since Github allows username changes so if there is a new repo, and someone changed their Github username in the meantime, the invitation could fail (best case) or actually end up going to the wrong person.

I think Github keeps old usernames as aliases for a few months, but the username itself is "free" and the alias is removed earlier if someone registers a new account with that username (IIRC).

So while existing repos would correctly retain references to the right Github accounts, keeping a manual list as a basis for auto-invites seems like it could go out of sync over time.

For SVN, restrictions by developer policy worked fine (I think?) and with Git, a lot of accidents would likely be prevented by just enforcing linear history and preventing merge commits (like that huge merge commit which made Phab go on an email sending frenzy).
Comment 5 Tom Stellard 2019-10-10 21:42:14 PDT
(In reply to Jonas Hahnfeld from comment #3)

> Is there anything else that "normal" contributors should not be allowed to
> do and that cannot be disabled?

Thanks for compiling the list of differences this was very helpful.

Other differences I found are listed here:
https://help.github.com/en/articles/converting-an-organization-member-to-an-outside-collaborator

I do agree that teams seem easier to manage.  I still like having collaborators at this stage, because they default to the lower permissions.  However, I would like to revisit this after the migration.
Comment 6 Tom Stellard 2019-10-10 21:50:47 PDT
(In reply to Kristina Brooks from comment #4)
> (In reply to Jonas Hahnfeld from comment #3)
> > 
> > Not right now, but just in case there will be a new repo in the future, do
> > you want to add all contributors manually / with a script? Just ticking a
> > box for a whole team would be much easier.
> >
> 
> That does seem like a better way since Github allows username changes so if
> there is a new repo, and someone changed their Github username in the
> meantime, the invitation could fail (best case) or actually end up going to
> the wrong person.
> 
> I think Github keeps old usernames as aliases for a few months, but the
> username itself is "free" and the alias is removed earlier if someone
> registers a new account with that username (IIRC).
> 
> So while existing repos would correctly retain references to the right
> Github accounts, keeping a manual list as a basis for auto-invites seems
> like it could go out of sync over time.
> 

If we need to make mass changes to commit permissions in GitHub in the future, we will pull the list of collaborators from GitHub and not from the manual file we have now in SVN.  This file is just an easy mechanism for us to verify the GitHub accounts of existing SVN users, and not something that we will use forever.


> For SVN, restrictions by developer policy worked fine (I think?) and with
> Git, a lot of accidents would likely be prevented by just enforcing linear
> history and preventing merge commits (like that huge merge commit which made
> Phab go on an email sending frenzy).
Comment 7 Jonas Hahnfeld 2019-10-11 00:26:57 PDT
(In reply to Tom Stellard from comment #5)
> (In reply to Jonas Hahnfeld from comment #3)
> 
> > Is there anything else that "normal" contributors should not be allowed to
> > do and that cannot be disabled?
> 
> Thanks for compiling the list of differences this was very helpful.
> 
> Other differences I found are listed here:
> https://help.github.com/en/articles/converting-an-organization-member-to-an-
> outside-collaborator

Unless I'm missing something, all points are included in my list above (plus some).

> I do agree that teams seem easier to manage.  I still like having
> collaborators at this stage, because they default to the lower permissions. 
> However, I would like to revisit this after the migration.

Your call, but I don't really get the point of not doing it right from the beginning (without disadvantages that cannot be easily avoided). Especially if every committer needs to accept 3 invitations (llvm, test-suite, zorg) and will get more notifications in the future about becoming a team member and being kicked out of repos after becoming so.