Proposal: Automatically requesting code reviews from plugin developer teams (CODEOWNERS)

[Oleg Nenashev]

Hi all,

I propose to improve the code review process across the Jenkins GitHub organization. TL;DR: Let's introduce CODEOWNERS in repositories and automatically request reviews from maintainer teams.

Motivation: In a number of plugins we have issues with pull requests which do not get timely reviews from the maintainers. It slows down delivery of fixes and impacts contributor experience, especially for newcomers who have to wait and to ping maintainers. Finally it impacts our ability to attract and retain contributors, and also causes frustration among maintainers and Jenkins users who see the desired PRs unmerged.

Why does it happen? We have well known issues with abandoned plugins, and there we cannot do much except promoting the adoption process. But lack of reviews also happens in other plugins. In many cases it is just caused by maintainers missing GitHub notifications (I am guilty of that too) and/or forgetting to follow-up. It is normal, because many plugins are maintained by volunteers. Life happens, work happens, etc. But we could help maintainers to keep track of review requests.

Current state: 

• In recent years GitHub introduced support of code review requests. GitHub offers built-in dashboards so that every user can see the pending reviews (link). There are also tools like In recent years GitHub introduced support of code review requests. Pull reminders which can integrate with corporate environments. For example, it allows to notify about new PRs and to periodically remind about stale review requests in Slack.
• How do we use GitHub? For each plugin we already have a GitHub team ( ${reponame}-developers) which could be used to request reviews (see GitHub permissions management). jenkinsci organization members can request reviews inside the organization
• External contributors cannot request reviews if they are not a part of the organization or repository collaborators. They can only CC maintainers in comments, and they won/t be able to see developer teams and request reviews from them

Proposed solution: GitHub now offers Code Owners metadata file for repos. It allows to specify owners of particular sections of code and to automatically request reviews from them in pull requests. Such reviews will be requested even if the submitter is not a member of the GitHub organization. It would also help organization members, because they will not need to manually request reviews and spend time on it. In order to implement that for a repo, we just need to add a string like "* @jenkinsci/pluginId-plugin-developers " in to .github/CODEOWNERS (example). 

Scope of changes: Plugin repositories inside the jenkinsci GitHub organizations. Other organizations (e.g. jenkins-infra) or non-plugin repositories are out of the scope.

Risks:

• "${reponame}-developers" team is a common practice, put it is not a case for all plugin repositories.
• Solution: We skip repositories with a different permission model
• Not every maintainer may want to be requested in such way. Some people do not like to receive too many notifications, and prefer to look at the repository periodically.
• Solution: the process should be opt-in
• Ownership changes in plugin? How they will impact the process
• Solution: we use a "${reponame}-developers" team, so that the process is not bounded to individuals. Once a new contributor added to the list of plugin maintainers, he/she will receive review requests for newly created PRs and review re-requests

Rollout plan:

• Jenkins project recommends setting CODEOWNERS in the repositories
• We add CODEOWNERS template to plugin archetypes 
• We submit pull requests to plugin repositories which have associated  "${reponame}-developers" teams. Due to the number of repositories it will likely require a bot, similar to how Daniel Beck handled the Plugin POM HTTP/HTTPs mess cleanup
• Each plugin maintainer or maintainer team will decide on their own whether they accept the process or not. Merging or closing the pull request will indicate the decision

I think that such change could greatly improve contributor experience across the  in the project. What do you think?

Thanks in advance,

Oleg Nenashev

 

[Mark Waite]

+! from me.  I like that idea very much.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAPfivLDCPCEb3oE_4uynf%2BE8KcFcSaY5pxy3MR6wveR%2BdtijBw%40mail.gmail.com.

--

Thanks!

Mark Waite

[Jesse Glick]

On Thu, Jan 9, 2020 at 8:05 AM Oleg Nenashev <o.v.ne...@gmail.com> wrote:
> We add CODEOWNERS template to plugin archetypes

Yes, though we need to do JENKINS-58557 first.

> We submit pull requests […]

> Each plugin maintainer or maintainer team will decide on their own whether they accept the process or not. Merging or closing the pull request will indicate the decision

Perfect, this makes the barrier to acceptance quite low, without
forcing a change on anyone.

[Oleg Nenashev]

Perfect, this makes the barrier to acceptance quite low, without forcing a change on anyone. 

Yes, I do not think we are in position to enforce the process at the moment. Historically Jenkins project gives a lot of freedom to plugin maintainers to define how they operate, and IMHO it is a part of the project's culture. IMHO we should rather lead by example there and help by providing tools and solutions simplifying the work and removing routine.

On Thursday, January 9, 2020 at 3:04:51 PM UTC+1, Jesse Glick wrote:

[Gavin Mogan]

Sweet I was going to suggest the same thing..

Lots of plugin maintainers don't realize they are not "watching" a repo. and I think tools like pr bot don't count you unless your listed as a reviewer

So much +1 for me

--

You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/f3f0b2d4-bb98-458d-9c96-ccdee1d1fec1%40googlegroups.com.

[Oleg Nenashev]

I'd guess any plugin maintainer can just proceed and add CODEOWNERS to the repo.

There is no need to wait for the bot if you want to start adopting the suggestion :)

On Thursday, January 9, 2020 at 7:07:08 PM UTC+1, Gavin Mogan wrote:

Sweet I was going to suggest the same thing..

Lots of plugin maintainers don't realize they are not "watching" a repo. and I think tools like pr bot don't count you unless your listed as a reviewer

So much +1 for me

On Thu., Jan. 9, 2020, 6:29 a.m. Oleg Nenashev, <o.v.n...@gmail.com> wrote:

Perfect, this makes the barrier to acceptance quite low, without forcing a change on anyone. 

Yes, I do not think we are in position to enforce the process at the moment. Historically Jenkins project gives a lot of freedom to plugin maintainers to define how they operate, and IMHO it is a part of the project's culture. IMHO we should rather lead by example there and help by providing tools and solutions simplifying the work and removing routine.

On Thursday, January 9, 2020 at 3:04:51 PM UTC+1, Jesse Glick wrote:

On Thu, Jan 9, 2020 at 8:05 AM Oleg Nenashev <o.v.n...@gmail.com> wrote:
> We add CODEOWNERS template to plugin archetypes

Yes, though we need to do JENKINS-58557 first.

> We submit pull requests […]
> Each plugin maintainer or maintainer team will decide on their own whether they accept the process or not. Merging or closing the pull request will indicate the decision

Perfect, this makes the barrier to acceptance quite low, without
forcing a change on anyone.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to jenkin...@googlegroups.com.

[Oleg Nenashev]

One thing here is that the most of the pluginId-developers teams are secret at the moment, and the teams must be public to operate properly with CODEOWNERS.

I suggest to make all of them public, IMHO there is no value in hiding teams in the open-source project (apart form unintended mention prevention).

I will add it to the next governance meeting, but I would appreciate the feedback

You received this message because you are subscribed to a topic in the Google Groups "Jenkins Developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/jenkinsci-dev/U_lFkTzmg1E/unsubscribe.
To unsubscribe from this group and all its topics, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/bc4c21c0-a18c-467a-a02e-a26cfa5acd97%40googlegroups.com.

[Marky Jackson]

Oleg,

I feel that need some form of consent from maintainers. 

On Jan 13, 2020, at 4:32 AM, Oleg Nenashev <o.v.ne...@gmail.com> wrote:



To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAPfivLATCB1R_OWLsEuGNoCx-k%2BqfQwJHxwvJOVbEFkgHWTW4A%40mail.gmail.com.

[Tony Noble]

Quite happy for that in the case of the plugins I maintain.  My details are in the POMs anyway.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/77113232-CBF5-4E20-963D-836E75DA4161%40gmail.com.

[Slide]

I think this proposal sounds good. The groups are created automatically by the bot, so its not like its a thing we are trying to hide anyway. Is there anything we need to do in the bot to make sure these groups are public going forward as new plugins are hosted?

To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAPfivLATCB1R_OWLsEuGNoCx-k%2BqfQwJHxwvJOVbEFkgHWTW4A%40mail.gmail.com.

--

Website: http://earl-of-code.com

[Tim Jacomb]

The list of maintainers is already public in the repository permissions list,

+1 for changing this

I assume the API call needs to be updated slightly in the IRC bot

Thanks

Tim

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/77113232-CBF5-4E20-963D-836E75DA4161%40gmail.com.

[Marky Jackson]

But the teams are not, and that could expose individuals without their consent. 

On Jan 13, 2020, at 4:58 AM, Tim Jacomb <timja...@gmail.com> wrote:



To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAH-3Bice%2BvxVnd%3DvidbOeorNhxvpyNixrvVEGNqcEHREoz-zyw%40mail.gmail.com.

[Slide]

Looks like the github-api library does not currently support setting the `privacy` value for a team.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAH-3Bice%2BvxVnd%3DvidbOeorNhxvpyNixrvVEGNqcEHREoz-zyw%40mail.gmail.com.

--

Website: http://earl-of-code.com

[Slide]

The default for a team is "secret" which means members are not visible to anyone, the only other option available is "closed" which means the members are visible only to members of the organization, not the general public. I think since it is not exposed to the general public that this should be ok?

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/F6FD3C9F-6EE2-48B7-96FC-754CC39E4EEF%40gmail.com.

--

Website: http://earl-of-code.com

[Daniel Beck]

How do you plan to address issues in plugins whose maintainers are largely (or even completely) managed outside the repo-specific team, or with custom teams that are largely maintained manually?

Since we grant repo admin permissions, it's easiest for maintainers to just add new people as external collaborators, rather than go through Jira to get someone to tell the IRC bot to add a new team member. Nobody is a team maintainer by default.

As an example of the kind of mess we're in, see jenkinsci/blueocean-plugin which has more than a dozen external collaborators, and its blueocean-plugin Developers team governs write access to four separate repositories (which also have external collaborators, and possibly their own "$pluginId-plugin Developers" teams).

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAPfivLDCPCEb3oE_4uynf%2BE8KcFcSaY5pxy3MR6wveR%2BdtijBw%40mail.gmail.com.

[Gavin Mogan]

I think blueocean should always be treated as the exception not the rule. I tried to do the codeowners for it and didn't realize the privacy thing. https://github.com/jenkinsci/blueocean-plugin/blob/master/CODEOWNERS

Codeowners would be opt in, and maybe even defaulted. But not mandatory. I think if the code owners with teams worked, more people would update their teams instead of individuals.

And if they don't. Then the plugins are no worse off than if we did nothing.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAMo7PtLEk5Sg6nKBZ%3Dp5Ghg0SqYhx6C7uuNiT7dCrtTcKvBZoA%40mail.gmail.com.

[Daniel Beck]

On Mon, Jan 13, 2020 at 2:43 PM 'Gavin Mogan' via Jenkins Developers <jenkin...@googlegroups.com> wrote:

I think blueocean should always be treated as the exception not the rule.

There are currently around 300 outside collaborators to jenkinsci repositories (compared to 2000 members), with another 50+ invitations pending. That's not all blueocean-plugin, I just knew of that one.

[Tim Jacomb]

Possibly could give maintainers team maintainer permission to help with that if the person they want to add is a member of the org

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAMo7PtLEk5Sg6nKBZ%3Dp5Ghg0SqYhx6C7uuNiT7dCrtTcKvBZoA%40mail.gmail.com.

[Oleg Nenashev]

Possibly could give maintainers team maintainer permission to help with that if the person they want to add is a member of the org

We usually grant Admin permissions to new teams so that they can manage their GitHub apps & Co.

We started doing that before Maintainer permissions were introduced, and now it might make sense to reconsider that and grant Maintainer instead.

Unrelated to this thread tho 

On Monday, January 13, 2020 at 6:44:34 PM UTC+1, Tim Jacomb wrote:

Possibly could give maintainers team maintainer permission to help with that if the person they want to add is a member of the org

On Mon, 13 Jan 2020 at 13:37, Daniel Beck <db...@cloudbees.com> wrote:

How do you plan to address issues in plugins whose maintainers are largely (or even completely) managed outside the repo-specific team, or with custom teams that are largely maintained manually?

Since we grant repo admin permissions, it's easiest for maintainers to just add new people as external collaborators, rather than go through Jira to get someone to tell the IRC bot to add a new team member. Nobody is a team maintainer by default.

As an example of the kind of mess we're in, see jenkinsci/blueocean-plugin which has more than a dozen external collaborators, and its blueocean-plugin Developers team governs write access to four separate repositories (which also have external collaborators, and possibly their own "$pluginId-plugin Developers" teams).

To unsubscribe from this group and stop receiving emails from it, send an email to jenkin...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAPfivLDCPCEb3oE_4uynf%2BE8KcFcSaY5pxy3MR6wveR%2BdtijBw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to jenkin...@googlegroups.com.

[Tim Jacomb]

> The default for a team is "secret" which means members are not visible to anyone, the only other option available is "closed" which means the members are visible only to members of the organization, not the general public. I think since it is not exposed to the general public that this should be ok?

This is implemented here:

https://github.com/jenkins-infra/ircbot/pull/75

I've also implemented support to make transition easier, allow switching a secret team to closed and allow upgrading permissions to maintainer of the team,

1. New teams created will be 'visible' (org members can see them and they show up in the reviewers list if CODEOWNERS are used)
2. The first user in the HOSTING request will be made a 'Maintainer' of the GH team
3. Adds a new command for making a secret team visible
4. Adds a command to add a user as a maintainer of a team

Any feedback welcomed here or in the PR

Thanks

Tim

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAPiUgVdLn9zs9ze4hHAPW%3DktKSxuao_S%3D2WSpxxZR2aUaxSxow%40mail.gmail.com.