RFC 37: Split META.yml suggested_reviewers into suggested_reviewers and notify

web-platform-tests / rfcs

web-platform-tests RFCs

75 stars 63 forks source link

RFC 37: Split META.yml suggested_reviewers into suggested_reviewers and notify #37

Closed zcorpan closed 3 years ago

zcorpan commented 4 years ago

See the rendered version of this RFC.

foolip commented 4 years ago

@zcorpan by happenstance I noticed that the GitHub API has a requested_teams field on PRs, and that indeed it's possible to request review from a team. Can you check what this does in terms of notifications and email filtering? I don't think that it's a solution to any of our problems, but if it does something different to requesting from everyone separately it might be worth considering.

zcorpan commented 4 years ago

(I haven't yet looked into requested_teams. I've made a todo to do so in early January.)

Having notify for "I'm interested in everything" could also allow for different delegation of suggested_reviewers.

For example, myself and @foolip are listed in html/META.yml, but for html/obsolete/META.yml only @foolip is. Right now the latter makes no difference; we're both notified for PRs in obsolete. An alternative would be for @wpt-pr-bot to only consider the nearest META.yml for suggested_reviewers, and all ancestor directories' META.yml for notify.

I don't know if this would be an improvement or a regression, though. Thoughts?

gsnedders commented 4 years ago

Pretty sure that would be a regression. I'm not opposed to some way to non-recursively add yourself as a reviewer/notify or a way to exclude certain children.

zcorpan commented 4 years ago

I've updated this to address comments. I still haven't looked into requested_teams.

stephenmcgruer commented 4 years ago

@zcorpan At this point, are you still interested in pursuing this RFC?

If not, we should determine whether there is appetite from another community member to champion this. It seems like there is enough interest that we shouldn't just drop it.

zcorpan commented 4 years ago

Unfortunately, I don't have bandwidth to work on this at the moment (or the next few months probably). I would be happy if someone else can take this. :)

stephenmcgruer commented 4 years ago

Thanks @zcorpan ; it can be hard to take the step back and acknowledge when one is overloaded, but it's also important so I appreciate you doing so :)

I have reviewed this thread and the RFC itself. Outside of the actual implementation work, it appears there is not that much currently to do here. I came up with the following list:

Determine what requested_teams does and if would be useful.
(Optionally) Consider other extensions that would be useful.

For the latter, I took a look through Chromium's OWNER files for inspiration. We have:

noparent (tells an OWNERS file not to recurse upwards),
per_file (accepts a glob-pattern for files a particular person cares about),
file (which appends another OWNERS file onto this one),
and a WATCHLIST concept which sort of separates our 'notify' and 'reviewer' sets into different files.

jgraham commented 3 years ago

It seems like this is not making progress currently and there's no one who is able to champion it. I'm going to close the issue for now, but we can reopen it if we identify a way forward.