-
Notifications
You must be signed in to change notification settings - Fork 55
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
Fixing semver checks again #384
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #384 +/- ##
=======================================
Coverage 77.66% 77.66%
=======================================
Files 49 49
Lines 10079 10079
Branches 10079 10079
=======================================
Hits 7828 7828
Misses 1805 1805
Partials 446 446 ☔ View full report in Codecov by Sentry. |
bf9d962
to
60863fe
Compare
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.
nice catch on the security concern!
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.
Moving PR: write to a just the labeler is nice. GJ
60863fe
to
6b84489
Compare
The saga continues.
Turns out
pull_request_target
triggers mean that thecheckout
action will checkout the head ofmain
, not the head of the PR, so the check was a no-op because it was comparingmain
tomain
and nothing changed.So, now we specify a
ref
in the checkout action and point it atgithub.event.pull_request.head.sha
which afaict will checkout the head of the PR.That means the running the job with write permissions would be dangerous because any code in the PR could be evaluated, so I've also split this into two jobs. One that does the check and needs the PR code, which runs with only read permissions, and then a second job that looks at the output of the previous job and just adds a label or not and nothing else, so it can have write permissions.
Tested as best I can via using a "normal"
pull_request
trigger. You can see the expected run here where it got as far as correctly wanting to add the label, but failing due to permissions (which will be fixed by merging this)