-
Notifications
You must be signed in to change notification settings - Fork 156
Add CHANGELOG.md entry checker to GHA workflows #500
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
Conversation
r? @ryankurte (rust-highfive has picked a reviewer for you, use r? to override) |
.github/workflows/changelog.yml
Outdated
@@ -0,0 +1,19 @@ | |||
on: | |||
pull_request_target: |
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.
Does it need to be pull_request_target
? On the example for the action they seem to just use a normal pull_request
.
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.
Not sure, it's a verbatim copy from the stm32f4xx-hal
. 🤷🏻♂️
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.
Could you try with just pull_request? If that works it's somewhat more straightforward and safer.
Thanks, looks like it works. Currently it won't stop bors at all (if I bors+ this now, it should merge, I think) because it doesn't run on bors' push to staging and also bors isn't configured to care about it. I think this sort of makes sense - it should alert reviewers/users that the changelog is missing, but doesn't stop a change being r+'d if the reviewer thinks it's fine. I don't think the "apply label to allow it to pass without changelog" would work under bors anyway, so this is probably the best config. What do you think? Happy to merge as-is. |
Signed-off-by: Daniel Egger <daniel@eggers-club.de>
4690429
to
be1eeef
Compare
Oh, sorry, I didn't notice the force push. What did you change? |
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.
bors merge
As requested I changed |
I agree. Alerting the reviewer to a missing CHANGELOG entry should be okay. |
r? @adamgreig
Signed-off-by: Daniel Egger daniel@eggers-club.de