Conversation
|
I'd rather hold on to mergify until we have better coverage, or somehow os-autoinst is integrated into the testing (or testing gets better in general) |
|
@foursixnine We'd still require 2 approvals and no pending reviews. |
okurz
force-pushed
the
feature/%ergify
branch
from
1ceaa74 to
e496d7e
Compare
Having low coverage didn't stop us or you from merging regression introducing changes in the past ;) I still consider it an important task by human reviewers to approve only after thorough review which should include manual testing as long as further automatic testing does not cover more. This merely codifies rules into automatic executions and also helps in case of trivial things, e.g. everyone tested and approved but there are just simple typos to fix or tidy rules to apply, etc. |
|
tidy rules seem to have updated. I need to do #44 first |
| - "check-success~=🐪 Perl" | ||
| # we don't need to spell out all tests but we can check a minimum | ||
| # amount | ||
| - "#check-success=>6" |
There was a problem hiding this comment.
Can you make it, so that it's the entire workflow?. Checking, only a few doesn't sound too reassuring to me. Still with two reviewers approving, should't be the end of the world.
There was a problem hiding this comment.
Well, I don't need to include all as we also check that none failed. So we basically only need to cover any potential short "racy" time where some checks would have succeeded already but others did not yet even show up as pending. Likely this can not even happen as long as we only have github actions. This would e.g. only apply for something external like travisCI or circleCI which would report back with a pending status only after some seconds or even minutes
| - name: automatic merge on special label | ||
| conditions: | ||
| - and: *base_checks | ||
| - and: | ||
| # mergify config checks needs at least two rules in "and" so we repeat | ||
| # one from the base checks | ||
| - base=master | ||
| - "label=merge-fast" |
There was a problem hiding this comment.
Is this really needed here? I mean for now fast merge makes sense, but in the future shuld be removed right?
There was a problem hiding this comment.
Well, the difference to the first rule is merely that we don't want to await approval from multiple persons. So for example for a urgent security fix or bugfix that you create I would set that label and as soon as all other checks have passed then mergify would merge regardless of the number of approvals. But I can give that label while tests are still running. mergify will still await successful tests before merging.
foursixnine
left a comment
foursixnine
left a comment
There was a problem hiding this comment.
Just for the record, it's not that I'm completely against mergify... it's that id prefer that merges happen when we know all is good.
I agree. I just want to give a proper written definition of what "all is good" means to us :) |
3 participants
No description provided.