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

🏗 A few minor OWNERS fixes #24971

Merged
merged 5 commits into from Oct 9, 2019
Merged

🏗 A few minor OWNERS fixes #24971

merged 5 commits into from Oct 9, 2019

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented Oct 8, 2019 •
edited

  • Remove default settings and unnecessary trailing comma from .prettierrc
  • Don't ignore OWNERS files in .eslintignore
  • Auto-fix a few OWNERS files (to be consistent with others)
  • Don't auto-request reviews from contributors who no longer work on AMP full-time

@rsimha rsimha self-assigned this Oct 8, 2019
@rsimha rsimha requested review from kristoferbaxter and honeybadgerdontcare and removed request for yuizumi and aghassemi October 8, 2019 21:32
@rcebulko
Copy link
Contributor

rcebulko commented Oct 8, 2019

Systax looks fine, but the changes in css, extensions, and tools change actual ownership structure, which I don't have authority to sign off on, so please loop in owners of those OWNERS files :)

@rcebulko
Copy link
Contributor

rcebulko commented Oct 8, 2019

Also this conflicts with #24960

@rsimha
Copy link
Contributor Author

rsimha commented Oct 8, 2019

Also this conflicts with #24960

Good to know. I'll let that get merged and undo any conflicting changes here.

@rsimha rsimha mentioned this pull request Oct 8, 2019
Merged
rcebulko
rcebulko reviewed Oct 9, 2019
View reviewed changes
css/OWNERS Show resolved Hide resolved
@rsimha rsimha merged commit c442731 into ampproject:master Oct 9, 2019
@rsimha rsimha deleted the 2019-10-08-OwnersPrettier branch October 9, 2019 17:16
@rcebulko
Copy link
Contributor

@rsimha ampproject/performance doesn't exist (should be wg-performance). Didn't catch this among all the other OWNERS changes. Also, looking at this retroactively, not sure if we want /runtime or /wg-runtime (both are teams).

@rsimha
Copy link
Contributor Author

rsimha commented Oct 16, 2019

ampproject/performance doesn't exist

Yikes, good catch. (Will it be feasible for the owners bot API check to flag this sort of error once it becomes available?)

not sure if we want /runtime or /wg-runtime

I think the preference is to use working group names. @choumx or @mrjoro can confirm. (Perhaps the runtime team can be removed in favor of wg-runtime?)

@mrjoro
Copy link
Member

mrjoro commented Oct 16, 2019 •
edited

Agreed; let's use only the wg- teams in Owners (other than Reviewers/Collaborators/Approvers)

I have an open task to remove a bunch of the redundant GitHub teams.

@rcebulko
Copy link
Contributor

ampproject/performance doesn't exist

Yikes, good catch. (Will it be feasible for the owners bot API check to flag this sort of error once it becomes available?)

Yes, it will validate this

@rcebulko rcebulko mentioned this pull request Oct 17, 2019
Merged
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
@rsimha
🏗 A few minor OWNERS fixes (ampproject#24971)
3055992
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@honeybadgerdontcare honeybadgerdontcare honeybadgerdontcare approved these changes

@rcebulko rcebulko rcebulko approved these changes

@kristoferbaxter kristoferbaxter Awaiting requested review from kristoferbaxter

Assignees

@rsimha rsimha

Labels
cla: yes WG: infra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@rsimha @rcebulko @mrjoro @honeybadgerdontcare @googlebot