Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce new one ACK carve-out rule #2627

Merged
merged 3 commits into from
Apr 1, 2024
Merged

Introduce new one ACK carve-out rule #2627

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ebf5b67
Update test script mention
tcharding Mar 22, 2024
42d02fb
Merge Refactor and One ACK carve outs
tcharding Mar 22, 2024
9b70c65
Introduce new one-ack carve out rule
tcharding Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 15 additions & 9 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,32 +144,38 @@ Current list of the project maintainers:
- [Riccardo Casatta](https://github.com/RCasatta)
- [Tobin Harding](https://github.com/tcharding)

#### Refactor carve-out
#### One ACK carve-out

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

A PR may be considered for merge if it has a single ACK and has sat open for at
least two weeks with no comments, questions, or NACKs.

#### One ACK carve-out

We reserve the right to merge PRs with a single ACK [0], at any time, if they match
any of the following conditions:

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



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


## Coding conventions

Expand Down