Can some projects configure auto addition of reviewers to changes?
As I observed that proposed changes can easily be ignored for months if the original submitter did not manually added anyone or missed to do the footwork of chasing cores on irc, I wonder if it would not be possible for *some* projects to enable a feature to auto-add reviewers.
On the dark side of the world I seen the "codeowners https://docs.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners" features being quite successful in assigning reviewers, as a simple file can define the pool of potential reviewers. Based on how the repository is configured it does randomly pick a limited number of candidates. Github mentions in their release notes from 2017 that their inspiration for that feature was the OWNERS file used by chromium team (gerrit users). I also seen that Wikimedia experimented something https://phabricator.wikimedia.org/phame/live/1/post/139/gerrit_now_automatically_adds_reviewers/ in that area, apparently there is a wiki page where people can mark them as available to be picked.
The watched project feature in Gerrit is useless for this purpose as it does not randomly pick some of a pool of possible candidates. It may be practical to use if there are projects with only 2-3 core reviewers.
There are two aspects I am looking to address, at least partially:
A) Improve random external contributors first-contribution experience - right now, even if they propose a perfectly valid change, there is high chance it will not get noticed. As you can imagine this lowers the chance they would try again, or to become an active contributor.
B) Spread the review burden on cores and avoid "punishing" those that are more active from being assaulted by review requests.
The current secret source for getting reviews on a project where you do not know anyone involves looking at recent changes to spot people that did reviews or at least have core powers and try to contact those. Even if this works or not, it puts extra pressure on active people. That is why I would see an automatic round-robin approach more appropriate.
As some may no longer be involved with the project, this could prove to be an opportunity to revise the core list, or to gently remove yourself from the list of cores.
C) How to deter people from adding reviewers before CI jobs report green?
I have no idea how to educate people not to add reviewers to changes that are not yet green. I know there are valid cases where this is needed, but likely is more like on less than one 1:5.
As no hard limit would work, maybe there is a way to display some kind of message in the UI, advising user not to assign reviewers until CI reports green. The same issue applies even to automatic reviewer assignment, which should not happen right after a review is created. Ideally this should happen when it passed CI, but it could also be ok if it would happen when a review is moved from WIP to ready-for-review. I personally use a default where any change I create is marked as WIP and this default can also be changed at server level if we want.
-- /zbr
Le 05/03/2021 à 10:16, Sorin Sbarnea a écrit :
<snip> I also seen that Wikimedia experimented something <https://phabricator.wikimedia.org/phame/live/1/post/139/gerrit_now_automatically_adds_reviewers/> in that area, apparently there is a wiki page where people can mark them as available to be picked.
Hello,
I wrote that Wikimedia blog post! Let me side track a bit to give a bit more information.
The context is Wikimedia has been struggling with code review for as long as we had more than a handful of people. Code review is always a hot topic and is causing pain all across the community with various degrees of experience. Some repositories are carefully watched by their maintainers, others have mostly inner team reviews and it might be hard to get a review if you are an outsider.
*git blame*
An idea we had was to automatically add code reviewer based on `git blame` (which I often relies on). That is how we have added the reviewers by blame plugin. Cause I was super ambitious I went with the default setting and it got enabled on every single repositories. The day after, people complained about being added to reviewers to a random set of patches that touched code that they once touched, most often unrelated to their review capabilities. The huge spam of emails caused us to roll back that deployment and forget about automatically adding reviewers based on blame. It just don't fit within our ecosystem.
I have updated the blog post to highlight we have disabled the reviewer by blame plugin almost immediately.
*homemade tooling*
The primary system we use is Reviewer bot. People register their interest on a wiki page https://www.mediawiki.org/wiki/Git/Reviewers https://www.mediawiki.org/wiki/Git/Reviewers for example I can get added for any changes made to a `tox.ini` file on any repo or to any changes made to `ci/*` gerrit repositories. I can't remember the details of the implementation, I think all changes are send to a mailing list, the bot fetches the email, processes them, fetch the wiki page and apply the rules.
The equivalent as a Gerrit plugin would be the reviewers plugin:
https://gerrit.googlesource.com/plugins/reviewers/+/refs/heads/master/src/ma... https://gerrit.googlesource.com/plugins/reviewers/+/refs/heads/master/src/main/resources/Documentation/config.md
We have it installed but I don't think it is used. Last time I checked there is no UI for it and people have to add themselves via a reviewers.config file under refs/meta/config. Needless to say you loose 99% of the audience at that point. Editing a wiki page is way simpler for us.
*attention set*
Google is well aware of the issue related to find reviewers, or at least highlighting when a change should be acted on among the list of changes one is a reviewer for. Gerrit 3.3 introduces a featured named attention set which is a bit of a turn based, when one does an action they can bring up to the attention of some of the reviewers while others are not.
So if Jane asks me to rewrite a test, she will bring my attention, I will amend my code send a patch and bring attention back to her. After a while once we both agree the change is now fine, we will bring it to the attention of the others. I really like that turn based review among a subset of the reviewers, that should cut a bit of spam or at least make the dashboard slightly more helpful.
The feature is better described at https://gerrit-review.googlesource.com/Documentation/user-attention-set.html https://gerrit-review.googlesource.com/Documentation/user-attention-set.html I have been exposed to it while contributing to Gerrit upstream and it is really an enjoyable system.
And all the above do not address the root causes:
* finding who can review code
* burden put on the shoulder of the few that can actually review, leading to a huge backlog which is most probably ignored as a result
It is a social problem really :-\
On 2021-03-05 01:16:24 -0800 (-0800), Sorin Sbarnea wrote:
As I observed that proposed changes can easily be ignored for months if the original submitter did not manually added anyone or missed to do the footwork of chasing cores on irc, I wonder if it would not be possible for *some* projects to enable a feature to auto-add reviewers.
[...]
The StarlingX community asked last year for us to add the reviewers plugin. I have WIP changes proposed for it at https://review.opendev.org/724913 and https://review.opendev.org/724914 but we were waiting for the dust to settle on the Gerrit 2.13->3.2 upgrade before moving forward on those. This is probably a good time to revisit them.
participants (3)
-
Antoine Musso
-
Jeremy Stanley
-
Sorin Sbarnea