Standardize and document steps wrt to GitHub teams and PR review procedure

[tlylt]

Please confirm that you have searched existing issues in the repo

Yes, I have searched the existing issues

Any related issues?

No response

What is the area that this feature belongs to?

No response

Is your feature request related to a problem? Please describe.

We probably don't need an extensive/stage-wise PR review pipeline, but I do think that it could be useful if we have up-to-date GitHub teams to allow for requesting mass PR reviews (more than 1 person)
So

• a cs3281-2023 team containing all latest students in cs3281 (including people of similar capacity)
• a cs3282-2023 team containing all latest students in cs3282 (including people of similar capacity)
• an active team (which is what we have in active-devs, though may not be entirely up-to-date)

Describe the solution you'd like

Probably simple documentation on the steps to achieve the above in our project management docs, including a ~ once-a-year maintenance process to review the team settings/make updates every year.

Describe alternatives you've considered

Best if we can automate the above via GitHub Actions or other tools.

Additional context

Suggestions/feedback on the plan.

[kaixin-hc]

I agree this is super useful. Though part of the difficulty is that even the 3282 team members (like myself) do not have access to update github teams with members... Good protection to have that kind of role-based access, but we would also need to amend role permissions since this is not something handled by most.

(And if it's only handled by whoever is the maintainer at the time + @damithc , I am not sure if it should be on the developer docs)

[kaixin-hc]

After looking at the way this is set up, I have the following suggestion for the next batch of mantainers like @lhw-1 -

The current organisation is 6 teams:

Admins have the admin role, contributors have the triage role, and developers have the write role (push access); the others have admin access. CS3282 is an old team (not current). CS32821 is a new team (I made it). Child teams seem to inherit the permissions of the parent but do not have to inherit the members of the parent; the members seem totally independent. I am not sure if pinging the parent group pings the child group but I would think not.

CS3281 students gradually gain more access to the repository as they become more familiar with it, so it is easier to make them part of a team and adjust the permissions for the whole group as the timeline dictates. I would keep this team and something to create fresh every year when there are multiple students to manage in this way.

Admin probably shouldn't be touched aside from adding at least one person who is more active to the group (and perhaps if @damithc wants to remove those who are not active and move them to a different group)

Active devs is useful and I think all 3282 members should be made mantainers of that team so they can make sure it is updated with new active devs. I have made this change to permissions. But since this is a child of contributors, you don't get push access from being in this group, and 3282 students need that.

I suggest that developer is given maintainer access and CS3282 students are placed under developer; contributor can be used for new contributors, driveby contributors, and non-active alumni.

[damithc]

(and perhaps if @damithc wants to remove those who are not active and move them to a different group)

Removed.

[tlylt]

@kaixin-hc I have summarized and adjusted the plan according to your comment, perhaps can help verify/let me know what you think?

I plan to remove the dedicated team for cs3281/cs3282 to reduce the number of teams. Though creating a new team for each batch might be easier to upgrade the permissions, I think keeping the number of teams small and remove/add members to the teams with correct permission might be easier to follow. As for pinging purposes, I think we can just use 'active-dev' whenever we need everyone's attention, as long as we keep that team up-to-date

Teams

• admin: admin access, for members who have admin access to the markbind repo/org -> for Mentor/Team Lead
  • release managers: admin access, for 3282 students who will be making new releases
• senior developers: push access, renamed from developers -> for cs3282 students (past and present)
• contributors: triage access, for cs3281 students (past and present) and non-cs3281 active devs
  • active devs: triage access, for everyone active

Workflows:

• new contributors (non-cs3281): for more than a few contributions -> add to contributors's active dev team
• new cs3281 students: add to contributor and contributors' active dev team
• new cs3282 students: add to senior developer team and contributors' active dev team
• new release manager (from cs3282) -> add to admin's release managers team
• new team lead -> add to admin team

[kaixin-hc]

This makes sense to me as well!

[lhw-1]

@tlylt After reading through the issue chain, the suggested team set-up and workflow make sense!

I believe that the necessary changes to the current team set-up is yet to be done - shall we go ahead with it to test it out for this semester @tlylt @damithc? As of right now, we definitely need to update the teams (e.g. cs3281 and cs3282 developers teams are outdated), so it would be a good idea to implement the suggestions in this thread and assess its effectiveness at the end of the semester.

[tlylt]

shall we go ahead with it to test it out for this semester

Hi @lhw-1, im ok. I think you should have access to perform the updates?

so it would be a good idea to implement the suggestions in this thread and assess its effectiveness at the end of the semester.

Sounds good to me, once we have decided we probably should update the dev docs to note this down formally, and close this issue.

[damithc]

shall we go ahead with it to test it out for this semester @tlylt @damithc?

@lhw-1 I'm OK as well. Go ahead.

[lhw-1]

I've made the necessary changes - will be updating this thread & the dev docs accordingly at the end of the semester.

[lhw-1]

After some thought and moving people around into different teams, I have the following proposal:

Teams:

• admin: admin access, for mentors and team leads (selected cs3282 students)
  • release managers: admin access, for selected 3282 students who will be making new releases
• developers: push access, for cs3282 students (past and present)
  • junior developers: push access, for cs3281 students (after recess week)
• contributors: triage access, for all contributors to MarkBind
  • active contributors: triage access, for all active contributors

Initially I wanted to try this approach:

CS3281 students gradually gain more access to the repository as they become more familiar with it, so it is easier to make them part of a team and adjust the permissions for the whole group as the timeline dictates.

But given that GitHub child teams inherit the permissions of the parent teams, this means that CS3281 students will have to be placed in a completely new team separate from contributors and senior developers (with triage and push access respectively), as we don't want to change the permissions for non-CS3281 contributors to push access, or for senior developers to triage access.

So now we are following this approach:

I think keeping the number of teams small and remove/add members to the teams with correct permission might be easier to follow.

Workflows:

• Non-CS328X developers:
  • When a developer non-trivially contributes to MarkBind, they can be added into the contributors team.
  • When a developer contributes multiple times to MarkBind and is active, they can be added into the active contributors child team.
• CS3281 students:
  • CS3281 students will first be added into the contributors team before the semester begins (if they have contributed to MarkBind).
  • They will be added to the contributors team and the active contributors child team once the semester begins. This will allow them to be pinged.
  • At the end of the recess week, we often give more permissions to CS3281 students for them to start managing; this is when they will be added to the junior developers child team, with push access.
  • At the end of the semester, they will be moved from the junior developers child team to the developers team.
  • At the end of each CS3281 semester (and beginning of each CS3281 semester), the junior developers team should be empty.
• CS3282 students:
  • CS3282 students should begin in the developers team, following the above approach; a few (or all) from a batch will be selected to be mentors and/or release managers, and will be added to the admin and/or release managers teams accordingly.

@tlylt @kaixin-hc @damithc does this approach make sense? We could also potentially make it even clearer by renaming "developers" to "CS328X" and "junior developers" to "CS3281", since these two teams (with push access) would pretty much only be used for CS328X students (past and present).

[kaixin-hc]

Thanks for investigating! I support your proposed approach, with a few comments to consider:

I strongly favour developers/junior developers over cs328X, which is not as layperson friendly. Developer Vs Contributor also has the effect of suggesting that developers have a higher level of commitment, which corroborates. Also, if we ever encounter the rare situation where someone becomes an active contributor who should have push access without taking cs3282 or 3281, they aren't in a weirdly named group.

I have some concerns over active devs being a sub team of contributors, I think it will very easily be confused with the developer team. A rename to active contributors would prevent this.

What permissions are given + the workflow for moving people should be documented both in the team description as well as a convenient place on the developer guide.

[lhw-1]

I have some concerns over active devs being a sub team of contributors, I think it will very easily be confused with the developer team. A rename to active contributors would prevent this.

That makes sense & aligns with the "developer vs contributor" differences we are going for here. I've edited the team names in the previous comment to highlight this more clearly.

What permissions are given + the workflow for moving people should be documented both in the team description as well as a convenient place on the developer guide.

We'll definitely need to write down the timeline and the steps for doing so, to make sure that this is actively done by the new CS3282 students and the outgoing CS3282 students every batch!