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

build: add CODEOWNERS linter action #34739

Closed
wants to merge 1 commit into from
Closed

build: add CODEOWNERS linter action #34739

wants to merge 1 commit into from

Conversation

mmarchini
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Aug 11, 2020
@addaleax
Copy link
Member

Fwiw, the job currently seems to fail

@mmarchini
Copy link
Contributor Author

Forgot to remove the "not owned" check, and the other check is valid (deduplicating). Lemme update the PR

@mmarchini
Copy link
Contributor Author

Ahh, so here we go:

  • github_access_token needs access to Teams API. GITHUB_TOKEN (the default token used on Actions) can't access it apparently
  • We can use the bot token for that, which means we need to increase the token scope to include Team read permission (or Org read permission, not sure how granular it is) and use the pull_request_target event

I'll remove the onwers check (it checks if GH user exists or if Team exists). Will request another token on nodejs/admin and open a follow up PR to re-add owners check.

@mmarchini mmarchini mentioned this pull request Aug 11, 2020
Open
richardlau
richardlau reviewed Aug 11, 2020
View reviewed changes
.github/workflows/linters.yml
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: mszostok/[email protected]
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a policy on using third-party actions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that I'm aware of

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We definitely have it enabled because this action ran

@mmarchini
Copy link
Contributor Author

Landed in 8b818cf

mmarchini added a commit that referenced this pull request Aug 17, 2020
@mmarchini
build: add CODEOWNERS linter action
8b818cf 
PR-URL: #34739
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@mmarchini mmarchini closed this Aug 17, 2020
MylesBorins pushed a commit that referenced this pull request Aug 17, 2020
@mmarchini @MylesBorins
build: add CODEOWNERS linter action
0c62aac 
PR-URL: #34739
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@danielleadams danielleadams mentioned this pull request Aug 20, 2020
Merged
BethGriggs pushed a commit that referenced this pull request Aug 20, 2020
@mmarchini @BethGriggs
build: add CODEOWNERS linter action
9ca4b2a 
PR-URL: #34739
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau left review comments

@BridgeAR BridgeAR BridgeAR approved these changes

@devnexen devnexen devnexen approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mmarchini @addaleax @jasnell @devnexen @richardlau @BridgeAR @nodejs-github-bot