feat: create setting in merge widget to set status as pending in failing situations #15
Conversation
|
Thanks but I think that it should be more subtle than that. For example when another status is failed, it might as well be failed for this one as well. |
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
|
|
|
What do you think of my changes? |
jelbourn
left a comment
jelbourn
left a comment
There was a problem hiding this comment.
LGTM aside from some minor coding nits
functions/src/plugins/merge.ts
Outdated
| if(matchAny([newLabel], config.checks.requiredLabels) || matchAny([newLabel], config.checks.forbiddenLabels)) { | ||
| if(matchAny([newLabel], config.checks.requiredLabels) | ||
| || matchAny([newLabel], config.checks.forbiddenLabels) | ||
| || (getLabelsNames(labels).indexOf(config.mergeLabel) !== -1 && matchAny([newLabel], config.checks.requiredLabelsWhenMergeReady)) |
There was a problem hiding this comment.
Use .includes() instead of indexOf()?
(here and below)
functions/src/plugins/merge.ts
Outdated
|
|
||
| if(matchAny([removedLabel], config.checks.requiredLabels) || matchAny([removedLabel], config.checks.forbiddenLabels)) { | ||
| if( | ||
| matchAny([removedLabel], config.checks.requiredLabels) |
There was a problem hiding this comment.
Break this check into a function since it's used more than once?
functions/src/plugins/merge.ts
Outdated
| }); | ||
|
|
||
| if(missingLabels.length > 0) { | ||
| checksStatus.pending.push(`missing required label${missingLabels.length > 1 ? 's' : ''}: "${missingLabels.join('", "')}"`); |
There was a problem hiding this comment.
Could eliminate the ternary by phrasing this like
`Missing required labels: ${missingLabels.join(', ')}`
ocombe
force-pushed
the
pending-as-fail
branch
from
cb52519 to
d3f0c21
Compare
ocombe
force-pushed
the
pending-as-fail
branch
from
d3f0c21 to
9cab2fe
Compare
4 participants
No description provided.