feat(approvals-satisfied): comment when PR does not meet approvals #631
Conversation
src/helpers/approvals-satisfied.ts
Outdated
|
|
||
| if (!approvalsSatisfied) { | ||
| await createPrComment({ | ||
| body: 'PRs must meet all required approvals before entering the merge queue.\n\n' + logsJoined |
There was a problem hiding this comment.
This helper doesn't only run in the context of the merge queue, so we should make the message more generic. Maybe like
Required approvals not satisfied: <logsJoined>
and we could pass the existing body input into this helper so the merge queue invocation can provide a merge queue-specific message too
| core.info(logs.join('\n')); | ||
|
|
||
| if (!approvalsSatisfied) { | ||
| logs.unshift('Required approvals not satisfied:\n'); |
There was a problem hiding this comment.
never heard of unshift until now, but it is equal to ['Required approvals not satisifed', ...logs]!
src/helpers/approvals-satisfied.ts
Outdated
| { teams, users, number_of_reviewers = '1', required_review_overrides, pull_number }: ApprovalsSatisfied = {}, | ||
| approvalsNotMetMessage: string | undefined = undefined |
There was a problem hiding this comment.
Can we use the existing body input instead of this second argument for providing an optional comment body? approvalsSatisfied is a standalone action
src/helpers/approvals-satisfied.ts
Outdated
| } | ||
|
|
||
| export const approvalsSatisfied = async ({ | ||
| teams, | ||
| users, | ||
| number_of_reviewers = '1', | ||
| required_review_overrides, | ||
| pull_number | ||
| pull_number, | ||
| approvals_not_met_message |
There was a problem hiding this comment.
Can we rename to body and add this to the above interface?
|
🎉 This PR is included in version 1.58.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
📝 Description
🔗 Related Issues