Introduce new one ACK carve-out rule#2627
Conversation
In the One ACK carve out just say "test scripts" instead of `test.sh` because we re-named the test scripts recently.
The "One ACK carve-out" has 3 rules and then there is a separate "Refactor carve-out" that covers things that are not only refactoring - this makes it hard to reference the carve-outs in github because its a bit confusing. Merge the carve-outs into a single "one ACK carve-out" with multiple rules. Use rule 0 for the original refactor carve-out stuff because it makes the diff smaller and all good lists start with 0. Also remove mention of the refactor carve-out from rule 3.
Pull Request Test Coverage Report for Build 8494297684Details
💛 - Coveralls |
|
At the risk of seeming self important, policy changes like this require a few eyes, @stevenroose, @RCasatta, @sanket1729, @Kixunil, @TheBlueMatt, @elichai, @apoelstra - can a few of you guys please review to make sure I am are not pushing through unwanted policy. |
sanket1729
left a comment
There was a problem hiding this comment.
ACK 234f444fa47d99aff4ed59e7180109cd7f40cdcb. I believe this will boost velocity.
apoelstra
left a comment
There was a problem hiding this comment.
ACK 234f444fa47d99aff4ed59e7180109cd7f40cdcb SGTM. Unsure whether it should be just me, but I guess I am the one with the local CI box...
RCasatta
left a comment
There was a problem hiding this comment.
ACK 234f444fa47d99aff4ed59e7180109cd7f40cdcb
Kixunil
left a comment
There was a problem hiding this comment.
ACK policy change but I believe there's a typo. IDK if you want to force push and apply the policy right away. ;)
CONTRIBUTING.md
Outdated
There was a problem hiding this comment.
Oh, lol. as sorta works in a flowery way puts on monocle "I do say sir, a PR as [one which has] had previously two ACKs, shall get a single ACK".
But I think you're right that it was supposed to be has, which can be read in a normal tone.
I think it'd be fine to fix the typo and then one-ACK merge this, since the typo doesn't really affect the meaning, and for process changes it's really just concept ACKs that we're looking for.
Our merge process is being artificially slowed down because of a combination of: - Using merge-commit merging means PRs often have to be rebased with no changes but a different merge base (and force pushed). - Trivial changes, like fixing nits, are often force pushed also. - Force pushes invalidate ACKs - Our devs are spread around the world working at different times What this means is trivial force pushes often cause multi day delays in merging. To try and alleviate this problem introduce an additional rule to the One ACK carve-out so that Andrew can merge PRs that have previously been ack'ed by another dev and have only minimal changes. The definition of "trivial" is subjective which introduces a burden on Andrew to not merge stuff willy-nilly but also allows simple changes to the original PR (eg fixed nits that the original reviewer suggested).
9b70c65
|
Woops, fixed |
Update merge carve-out policy and introduce new rule.
From patch 3: