-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RepoKitteh-based selective CODEOWNERS enforcement on select subtrees #7423
Comments
makes sense. let me figure out the timing. |
@alyssawilk I think this will also be useful for date plane change monitoring and review enforcement. |
/assign @itayd |
description makes sense, i'm working on it. |
@itayd FWIW, if we can have the ability to extend this facility to other GitHub teams and paths, e.g. to monitor changes to |
@htuch i see no other way than making it generic :) it's not much work, it's just configuration. i got it almost ready btw, will prolly finish it tomorrow. |
@htuch is it ok if i label such prs with the team they mention? otherwise figuring out double-mentions on consecutive commits can be troublesome. |
@htuch sans ^, things are ready. I just need to test a bit more. For that it'd be super useful if you could create a testers team in envoy that includes me. thanks. |
@itayd looks great, this is exactly what we were after here I think. Can I confirm that the "must approve" checks are dynamic and shrink/grow/disappear based on PR and path? |
Per #7423: * When a PR contains changes to api/, @envoyproxy/api-shepherds are notified via a CC comment. * When a PR contains changes to api/udpa, @envoyproxy/udpa-wg is notified via a CC comment. * An additional GitHub check blocking merge is added that will only be marked as passing once an approval review is received from someone in @envoyproxy/api-shepherds. Risk Level: None. But if there is any issue, please revert. Testing: In test PRs under different users: #7659 Docs Changes: None. Release Notes: None Fixes #7423 Signed-off-by: Itay Donanhirsh <itay@bazoo.org>
@itayd I feel the comment is a little bit noisy, can we do following?
|
Sorry for not engaging earlier. I'm still in Israel, back monday.
…On Thu, Aug 8, 2019, 5:42 PM Matt Klein ***@***.***> wrote:
@lizan <https://github.com/lizan> FYI I already mentioned to @itayd
<https://github.com/itayd> the noise issue. Hopefully he will fix soon.
The draft PR suggestion is a great idea also.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7423?email_source=notifications&email_token=AADIE2YSHHPR67DT6ZTP5A3QDQWHHA5CNFSM4H4FWM32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD33256Y#issuecomment-519548667>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADIE2Z4BRORIIPSH6SPS53QDQWHHANCNFSM4H4FWM3Q>
.
|
As part of the stable API versioning initiative in #6271, we need to be able to ensure that @envoyproxy/udpa-wg has visibility on API changes and that @envoyproxy/api-shepherds sign-off on any API changes in the
api/
subtree.While we make use of CODEOWNERS in GitHub to allow more permissive management of extensions and the like, this mechanism is too coarse grained, since when we enabled mandatory CODEOWNERS sign-off along with b9a1b6e#diff-5daf978e60e03b3975b485d5f1b449dc, this resulted in all aspects of the Envoy tree requiring CODEOWNERS sign-off.
I think RepoKitteh could be a solution here. We would want the following behavior on
api/
api/
, @envoyproxy/api-shepherds are notified via a CC comment.api/udpa
, @envoyproxy/udpa-wg is notified via a CC comment.I think 1/2 can turned into a general notification mechanism. We could implement them independent of 3, which could also be a mechanism useful elsewhere in the code base.
@itayd WDYT? CC @mattklein123 @caniszczyk for CNCF visibility.
The text was updated successfully, but these errors were encountered: