-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 github action to block merging large files #57360
base: main
Are you sure you want to change the base?
Conversation
6bf68b2
to
e6a8009
Compare
c30ed36
to
549642b
Compare
Converted to draft because the skip logic doesn't seem to work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bash isn't my strength but if it works great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great!
Do you mind adding a small snippet that comments on the PR as well? It's a rather low hanging fruit:
jobs:
comment:
runs-on: ubuntu-latest
steps:
- uses: actions/github-script@v6
with:
script: |
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: '👋 Thanks for reporting!'
})
@jhchabran, thanks for the suggestion! Will add. I converted this back to draft because I couldn't get the "override this check" working, and I think we want a "break glass" option. Do you have recommendations on how to handle that? |
@camdencheek there's AFAIK no easy way to do that, because we either allow specific actors to bypass all checks or we don't. I think given the context of thir particular check, it should be fairly rare that someone has to break glass. And if they did, they can reach us on the support channel. So I'm in favour of adding it as is, and we could totally print out a message telling folks to do so in the GitHub action error, or if you want to take it further, it's not hard to post a comment from the action itself on the PR to really surface that. I'm fine with both. |
This adds a github action which checks that the PR doesn't add any files greater than 1MB. We've had a lot of large binaries or test files be committed recently, so this should help block those from making it to
main
, since once they're inmain
, they're in the history permanently.Test plan
See example failure here.