Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add wg-runtime to OWNERS of src/ #24753

Merged
merged 1 commit into from Sep 26, 2019
Merged

Add wg-runtime to OWNERS of src/ #24753

merged 1 commit into from Sep 26, 2019

Conversation

dreamofabear
Copy link

No description provided.

@amp-owners-bot
Copy link

Hey @rcebulko, these files were changed:

  • src/OWNERS.yaml

rcebulko
rcebulko approved these changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@rcebulko rcebulko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rcebulko rcebulko self-requested a review September 26, 2019 14:37
rcebulko
rcebulko suggested changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@rcebulko rcebulko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ammendment to my previous approval: Should wg-runtime own all files in all subdirectories under src, or just the files directly within src?

kristoferbaxter
kristoferbaxter approved these changes Sep 26, 2019
View reviewed changes
Copy link
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

@rcebulko
Copy link
Contributor

Also relevant: dvoytenko is an owner of the root amphtml directory, so he also implicitly owns this directory. By also listing him here, he is considered "equally relevant" to wg-runtime when reviewers are being selected; by contrast, if he is removed from src/OWNERS.yaml, he would have the ability to give owners approval but would not be the first pick when selecting reviewers for files under src. I'm not certain which the intention is, but wanted to raise this @choumx

@dreamofabear
Copy link
Author

wg-runtime should own all files recursively. We can let Dima adjust his own comfort level for review requests.

@dreamofabear dreamofabear merged commit a797f70 into ampproject:master Sep 26, 2019
@dreamofabear dreamofabear deleted the runtime-owners branch September 26, 2019 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kristoferbaxter kristoferbaxter kristoferbaxter approved these changes

@jridgewell jridgewell jridgewell approved these changes

@rcebulko rcebulko rcebulko approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dreamofabear @rcebulko @kristoferbaxter @jridgewell @googlebot