1782278 - mots format does not include review groups

Status: RESOLVED FIXED

(Conduit :: mots, enhancement, P3)

Description

[Dan Robertson (:dlrobertson)]

The mots.yaml file does not include a entry for review groups for a module. It would be nice to include groups like gfx-reviewers, necko-reviewers, etc in the mots yaml file. This could help individuals looking for potential reviewers at the Firefox Source Docs: Modules and would be helpful for a get_maintainers.pl-like tool.

Comment 1

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Zeid, would you mind if I add some reviewer groups to mots.yaml (I obviously don't know every reviewer groups but I do know some) and also submit a patch for https://github.com/mozilla-conduit/mots for that? This would be so nice for me to find reviewers as I need to change files in random components time to time.

Comment 2

[Mark Banner (:standard8)]

(In reply to Kagami [:saschanaz] (they/them) from comment #1)

(I obviously don't know every reviewer groups but I do know some)

In theory it is all the groups with "-reviewers" in their name in this list: https://phabricator.services.mozilla.com/project/query/active/

There is also this list, but I'm pretty sure that's a sub-set of the one in phabricator.

I'm pretty sure Zeid won't mind (I started to look at a patch a while ago, but didn't have time in the end).

Comment 3

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

(In reply to Mark Banner (:standard8) from comment #2)

(In reply to Kagami [:saschanaz] (they/them) from comment #1)

(I obviously don't know every reviewer groups but I do know some)

In theory it is all the groups with "-reviewers" in their name in this list: https://phabricator.services.mozilla.com/project/query/active/

Oh thanks, that's not too many 👍

There is also this list, but I'm pretty sure that's a sub-set of the one in phabricator.

Oh, so there's already a doc for this. But a machine-readable list would be even better 😁

Comment 4

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Let me assign myself in case Zeid doesn't mind. Ping me if you do!

Comment 5

[Zeid [:zeid]]

:saschanaz -- go for it, however keep in mind that we don't necessarily have the concept of a "reviewer" in mots.yaml, just the owners/peers who could be reviewers. This comes up enough that I think it would be helpful. One way to do it would be to add it to the meta field, since that field is also machine readable and does not have any specific format requirements. The docs can then pull from there.

Comment 6

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Nice, thanks for the guide!

Comment 7

[Zeid [:zeid]]

In the future if we need to promote it to its own field that should be fairly straightforward. It might make sense to ultimately have a reviewers field that takes references to anchors (both users and groups), similar to how the owners/peers fields work. Lastly, since this information is stored in Phabricator, it would be good to figure out a way to automate fetching this info from Phabricator, e.g., by correlating the paths in herald rules with modules.

Comment 8

[BMO Github Automation]

Attachment: [mozilla-conduit/mots] feat: add meta.review_group (bug 1782278) (#75)

Comment 9

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: Bug 1782278 - Add `review_group` to mots.yaml r=zeid

As the initial patch this only covers a small set of groups I'm 100% sure they match the MOTS entries. Others will follow with a review request to each group.

Comment 10

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Attachment: WIP: Bug 1782278 - Add more review groups

Depends on D184748

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88b549716dbf
Add `review_group` to mots.yaml r=zeid DONTBUILD

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/88b549716dbf