Sitelet https://github.com/Forostovec/rust-bitcoin/commit/6c4f5df3554f82d01e06879d38f27bb6a475d351
Skip to content

Commit 6c4f5df

Browse files
apoelstraapoelstra
committed
Merge rust-bitcoin#2627: Introduce new one ACK carve-out rule
9b70c65 Introduce new one-ack carve out rule (Tobin C. Harding) 42d02fb Merge Refactor and One ACK carve outs (Tobin C. Harding) ebf5b67 Update test script mention (Tobin C. Harding) Pull request description: Update merge carve-out policy and introduce new rule. - Patch 1: Fix stale test script mention - Patch 2: Merge current carve-outs into a single carve-out with multiple rules - Patch 3: Introduce new carve-out rule From patch 3: ``` Introduce new one-ack carve out rule 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). ``` ACKs for top commit: apoelstra: ACK 9b70c65 confirmed via range-diff that the commit everyone ACKed and this one differ only in `as` vs `has` Tree-SHA512: 41898e71e013ac70e41bb4624ce5e5055dc3e7a405dd73d3988f5b02ece104d7fad746203ce8d26a6a33f98b745010fc39e9a4bddb9bcf22267c942a4dac2028
2 parents 4163641 + 9b70c65 commit 6c4f5df

1 file changed

Lines changed: 15 additions & 9 deletions

File tree

‎CONTRIBUTING.md‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -150,32 +150,38 @@ Current list of the project maintainers:
150150
- [Riccardo Casatta](https://github.com/RCasatta)
151151
- [Tobin Harding](https://github.com/tcharding)
152152

153-
#### Refactor carve-out
153+
#### One ACK carve-out
154154

155155
The repository is going through heavy refactoring and "trivial" API redesign
156156
(eg, rename `Foo::empty` to `Foo::new`) as we push towards API stabilization. As
157157
such reviewers are either bored or overloaded with notifications, hence we have
158158
created a carve out to the 2-ACK rule.
159159

160-
A PR may be considered for merge if it has a single ACK and has sat open for at
161-
least two weeks with no comments, questions, or NACKs.
162-
163-
#### One ACK carve-out
164-
165160
We reserve the right to merge PRs with a single ACK [0], at any time, if they match
166161
any of the following conditions:
167162

168-
1. PR only touches CI i.e, only changes any of the `test.sh` scripts and/or
163+
0. PR has a single ACK and has sat open for at least two weeks with no comments,
164+
questions, or NACKs.
165+
1. PR only touches CI i.e, only changes any of the test scripts and/or
169166
stuff in `.github/workflows`.
170167
2. Non-content changing documentation fixes i.e., grammar/typos, spelling, full
171168
stops, capital letters. Any change with more substance must still get two
172169
ACKs.
173170
3. Code moves that do not change the API e.g., moving error types to a private
174171
submodule and re-exporting them from the original module. Must not include
175-
any code changes except to import paths. This rule is more restrictive than
176-
the refactor carve-out. It requires absolutely no change to the public API.
172+
any code changes except to import paths. Requires absolutely no change to the
173+
public API.
174+
4. PR has previously had two ACKs, had minimal changes, and gets a single ACK
175+
from Andrew. This call is subjective, gives extra privileges, but also
176+
requires extra responsibility/accountability (including running a bunch
177+
of local CI checks before merging) [1].
178+
179+
177180

178181
[0] - Obviously author and ACK'er must not be the same person.
182+
[1] - The aim is to reduce the burden of re-ACK'ing trivial changes and also
183+
alleviate the problem of devs spread around the world in different timezones.
184+
179185

180186
## Coding conventions
181187

0 commit comments

Comments
 (0)