Would it be better to disable the "blunderbuss" plugin?

[bobcatfish]

We have the prow blunderbuss plugin enabled for all projects (https://github.com/tektoncd/plumbing/blob/894797a2511178fd0e8f43e82e131983144bc28e/prow/plugins.yaml#L25) but for the projects that I've worked on it seems like we consistently ignore it, and in fact for TEPs in this repo, we're using assignees where we probably want to use reviewers because the reviewers list is noisy and not useful.

I'm wondering if we might be better off without it (at least for now)? We can always keep it on for any projects that are using it.

@afrittoli @vdemeester what do you think?

[bobcatfish]

(btw details about our enabled plugins and what they do are available at https://prow.tekton.dev/plugins)

[vdemeester]

On a side not, there is a bunch of plugins that seem to be enabled that we could disable : bugzilla, buildifier, golint, stage and maybe joke and project (as we don't use project in any project so far)

[vdemeester]

So blunderbuss is the automation of the assign plugin. assign allows us to us cc and assign command, and blunderbuss does "pick" some up from the OWNERs.

The blunderbuss plugin automatically requests reviews from reviewers when a new PR is created. The reviewers are selected based on the reviewers specified in the OWNERS files that apply to the files modified by the PR.

This allows us to even "pick" the reviewers from a specific set of people, if we use folder OWNER (as we do here in tektoncd/pipeline)

This is what happens on a default PR on tektoncd/pipeline so I wouldn't say that the project consistently ignore it. It sets a default set of reviewers to the PRs that can then review or ping/assign other contributors/maintainers. I found it very useful to be honest 😛

[bobcatfish]

@vdemeester do you find that folks are actually paying attention to what they are assigned to review? i know i'm not 😬

If I'm the only one ignoring it then I'm happy to change what I'm doing - but I'm under the impression this is mostly going unused

For example this PR in pipelines:

Or this one:

Or in community #359:

(btw im not saying we disable the "assign" plugin, just "blunderbuss")

Or maybe the answer is to clean up the owners lists and start asking all owners to pay attention to what they are assigned?

[vdemeester]

So I am definitely not ignoring them at least (because they appear in my notification as request review, I treat them before the others).

[vdemeester]

A bit of update :

• /auto-cc comes from blunderbuss but does it “manually”/on-demand (request reviewers)
• /uncc @pseudo to remove yourself from the requested reviewer

/cc and /un-cc are coming from the assign plugins, so if we disable the blunderbuss plugin, we can still do this “on-demand”. One positive aspect of blunderbuss is that, at least some collaborators should get the ping and re-direct to the appropriate person if need be. That potentially reduces the triage work. That said, if most of the requested reviews do not do that work, blunderbuss is moo and could be uninstalled yes.

[vdemeester]

Ah wait, /auto-cc comes from blunderbuss…

[wlynch]

I like auto-assigned PR assignees/reviewers - it helps maintainers distribute the review load, and gives contributors a point of contact for their PR. My worry if we remove the blunderbuss plugin is that PRs won't be looked at and/or we put a manual burden on a small group of people to triage and assign PRs as they come in.

As a reviewer I tend to struggle with 2 things with how auto-assigned reviewers work today:

1. What is the responsibility when I get assigned as a reviewer for something I don't have a lot of knowledge with. I think something that would be useful is a counterpart to https://github.com/tektoncd/community/blob/main/process.md#assigning-yourself-an-issue to detail how to remove yourself. This might also be a signal that auto assignments aren't as granular as they should be 🤔
2. What's my responsibility when I'm assigned as a reviewer, but someone else has jumped in and started reviewing? Is my review still required or optional? (I tend to lean towards optional).

I think in my ideal world automation would look something like:

• At least 1 person is assigned. Assignees are on the hook for reviewing the PR, or finding another person / removing themselves to reroll a reviewer if they feel unqualified to review the change. Additional assignees can be added by anyone, w/ the expectation that all assignees should lgtm/approve. At least 1 of these assignees should be a project OWNER.
• Reviewers aren't auto assigned by default, and are there mainly as a CC/FYI with no expectation of a sign off.

(if anyone is familiar with Gerrit - tl;dr GitHub Assignee == Gerrit Reviewer, GitHub Reviewer == Gerrit CC)

[bobcatfish]

and/or we put a manual burden on a small group of people to triage and assign PRs as they come in

my understanding is that this is much more what is already happening - or at least i believe @sbwsg does a lot of triaging (maybe build captains too?)

id love to have some automation around this - if it actually works and ppl are actually using it

[bobcatfish]

maybe backing this up with some stats, looking at pipelines here is the 7 day moving average of reviews in the last 2 months

maybe some other top reviewers can chime in on whether they are doing these reviews via what they are being assigned or something else? @pritidesai @jerop @afrittoli @sbwsg @GregDritschler @chmouel

[vdemeester]

Same on 6 month might be a bit better on usual reviewers.

I am not fully sure how to read those charts though. Is this taking into account reviews only (aka not single comment on a line or on the PR itself) ?

[ghost]

I look at every PR in Pipelines at least once just to check if I should review it in more detail or not. I base that decision on the content of the PR and totally ignore the assigned Reviewers.

[jerop]

I do this too

[pritidesai]

The email notifications I receive (with requested review) at least draws my attention even in an area that I might not be most familiar with. After spending about 10 minutes on it and not fully understanding the changes (sometimes I end up asking dumb questions 😜 ), I push it to a back burner in my personal TODO and visit back if its open for more than a few days.

Apart from requested review, like @sbwsg and @jerop mentioned, every morning/afternoon I go through the pipeline repo for any new PRs and decide whether I should be reviewing it or not. I do ignore the assigned reviewers for most of the PRs but the /cc feature confuses me. Most of the times, I do not understand what that /cc mean? Is it that the folks cced are required to review before merging since they might have more insight into the changes? Or its just to draw their attention and its ok for non-cced folks to leave review and merge eventually?

Thanks @bobcatfish for initiating this discussion?