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.

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

ci: add copywrite action to check file headers. #17889

Merged
merged 2 commits into from
Jul 12, 2023
Merged

ci: add copywrite action to check file headers. #17889

merged 2 commits into from
Jul 12, 2023

Conversation

jrasell
Copy link
Member

@jrasell jrasell commented Jul 11, 2023

@jrasell jrasell added backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line labels Jul 11, 2023
@jrasell jrasell requested review from tgross and gulducat July 11, 2023 12:12
@jrasell jrasell self-assigned this Jul 11, 2023
@jrasell jrasell added the backport/1.6.x backport to 1.6.x release line label Jul 11, 2023
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

This LGTM but will it work in the ENT repo without needing special creds? @gulducat might be able to answer that

Copy link
Member

@gulducat gulducat left a comment

Choose a reason for hiding this comment

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

👍

this doesn't need backports though, does it?

Comment on lines +18 to +19
permissions:
contents: read
Copy link
Member

Choose a reason for hiding this comment

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

Though I don't know the full extent of what the copywrite command does, I imagine it should be fine. The contents of the repo itself are pulled by actions/checkout in the first step, which only needs this contents:read here.

Beyond that, according to the copyright repo's readme, for some operations

A GITHUB_TOKEN environment variable can be used with a Personal Access Token

I suspect the GITHUB_TOKEN (auto-provided by GHA), if used at all in our case, will mainly be to uniquely identify the caller so our private runners don't get some egress-IP rate-limited or something by GH API.

@tgross tgross removed the backport/1.3.x backport to 1.3.x release line label Jul 12, 2023
@tgross
Copy link
Member

tgross commented Jul 12, 2023

this doesn't need backports though, does it?

I'd have said yes but then I actually checked the release/1.5.x branch and it turns out we didn't backport all the copyright headers. So I think you're right we don't need them here.

@jrasell jrasell removed backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line labels Jul 12, 2023
@jrasell jrasell merged commit 74335b3 into main Jul 12, 2023
@jrasell jrasell deleted the f-gh-17887 branch July 12, 2023 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ci: add copyright and license header check to CI runs
3 participants