phab reviews

[Ben Cooksley]

On Fri, Aug 25, 2017 at 8:59 PM, Harald Sitter <sitter at kde.org> wrote:
> Hola!

Hi Harald,

>
> Our phabricator guide says that at least one person needs to be set as
> reviewer to get any review.
> https://community.kde.org/Infrastructure/Phabricator#Posting_a_Patch
>
> This sucks.

That is correct. Reviewboard had a similar issue, albeit with the lack
of any ability to autosubscribe people.

>
> For one it makes it hard to keep on top of reviews across all our software.
> More importantly though, how exactly do we expect a drive-by
> contributor to figure this out? I work in the community for years and
> yet often enough struggle to find a relevant person to set as
> reviewer.  Not just figuring out their name on phab, but even finding

You can search by someone's actual name in any autocomplete prompt
where names are accepted I believe, so not knowing their username
shouldn't be an issue.

> out who is relevant in the context of the repo. There are projects who
> have no real prominent head so looking at the git log basically gives
> you a bunch of drive-by commits.
>
> Can we improve this somehow?
>
> Not particularly enjoyable but working solutions I have in mind:
>
> a) establish phab team of volunteers to get spammed by all reviews (or
> more ideally reviews without reviewer set) and then either sort out
> the best reviewer or review it themself
>
> b) perhaps more scalable: no automatic subscription of the team but we
> change the wiki to tell the submitter to use the aforementioned team
> as reviewer when he doesn't know any better person

Phabricator has two different tools which can help us here: Herald and Owners.

Owners is a utility which allows someone to say what paths in a given
repository they are responsible for. These packages can then be either
used by themselves, or utilised by Herald rules.

Herald is an extremely powerful utility, which has the power to do ...
way too many things to list here.

One of those things is the ability to automatically subscribe Projects
or Individuals to reviews, or add them as reviewers or blocking
reviewers. Depending on how a given Herald rule is setup, it can
enforce this - escape is impossible.

Please see https://secure.phabricator.com/book/phabricator/article/owners/
and https://secure.phabricator.com/book/phabricator/article/herald/
for more detail on what they're capable of.

There is a performance penalty to be paid with Herald however of
around 1ms per rule we have, which is why access to Herald is
restricted to Community Admins.

We currently have rules to cover: Okular, Dolphin & Konqueror, All
Games, Kile, Konsole, Simon, Documentation (all repositories),
Frameworks, KWin, Plasma, KWayland, Kirigami, Ark, Kate, KProperty,
KDb, Calligra, Kexi, Plasma, Marble, KDE PIM, All Utils Apps, KDevelop
and RKWard.

These rules essentially CC the mailing list for those projects, and in
some instances add the appropriate project as a reviewer. At the very
least it should ensure it's brought to the attention of involved
developers.

>
> HS

Regards,
Ben