docs/RFCS: Update RFC process

[rytaft]

Update the RFC process to require a minimum of one week
of PR review before starting the Final Comment Period.
Add "postponed" as a possible RFC status.

[cockroach-teamcity]

This change is 

[tbg]

Looks good, thanks!

[danhhz]

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

   attempt to compensate by a serious review of alternatives.**

3. Go through the PR review. This process should last at least one

Was there something concrete that motivated this? It's a bit heavyweight for small rfcs, so I'm worried it'll discourage people from writing them. I'm also, in general, hesitant to add process unless there is a clear need.

(Relatedly, I think this was also part of the motivation of the FCP.)

---

Comments from Reviewable

[knz]

thanks!

[petermattis]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

docs/RFCS/README.md, line 55 at r1 (raw file):

   When the dust on the PR review has settled,
   and if there is consensus to proceed with the project, begin the
   final comment period (FCP) by (1) posting a comment on the PR and

@knz Have we ever had a comment appear during the FCP?

---

Comments from Reviewable

[rytaft]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, danhhz (Daniel Harrison) wrote…

Was there something concrete that motivated this? It's a bit heavyweight for small rfcs, so I'm worried it'll discourage people from writing them. I'm also, in general, hesitant to add process unless there is a clear need.

(Relatedly, I think this was also part of the motivation of the FCP.)

This was motivated by the discussion in PR #19577 - I started the FCP after I'd gotten a few LGTMs in a couple of days. @knz implied that waiting one week was sort of an unwritten rule, so I suggested adding it to the guidelines to make it explicit.

I thought PR #19577 was going to be a small RFC going in, but then it created a lot of discussion....

---

docs/RFCS/README.md, line 55 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

@knz Have we ever had a comment appear during the FCP?

PR #19577 was already in the FCP when it got a bunch of comments.

---

Comments from Reviewable

[petermattis]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

docs/RFCS/README.md, line 55 at r1 (raw file):

Previously, rytaft wrote…

PR #19577 was already in the FCP when it got a bunch of comments.

Ah, good point.

---

Comments from Reviewable

[danhhz]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

docs/RFCS/README.md, line 51 at r1 (raw file):
Gotcha, thanks. I agree with documenting our implicit assumptions. I happen to disagree with @knz's comment that this has been true in the past, but I'll get on board with making it true in the future if I'm in the minority.

I thought PR #19577 was going to be a small RFC going in, but then it created a lot of discussion....

Yeah, this is exactly why I want to keep from discouraging people from writing small rfcs. We would have had a worse outcome if you'd tried to skip the rfc process because you thought it was too heavyweight. We're already starting to get that sentiment: #19652 (comment)

---

docs/RFCS/README.md, line 55 at r1 (raw file):

Previously, petermattis (Peter Mattis) wrote…

Ah, good point.

Partitioning, too

---

Comments from Reviewable

[bdarnell]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

docs/RFCS/README.md, line 51 at r1 (raw file):
To the extent that we have observed an "unwritten rule" of a week before starting the FCP, I think that has been entirely by accident.

(Relatedly, I think this was also part of the motivation of the FCP.)

Yeah, this is what the FCP was supposed to be for, to ensure that there was a reasonable time for comments to come in. Maybe that time isn't long enough, but we should be wary of making this too heavyweight. A 9-day minimum seems too long to me.

With a shorter process, we won't be able to get feedback from everyone, and that's OK. The RFC author and a suitable primary reviewer should be able to agree and move on to implementation within a few days. We'll make mistakes (I was too quick to LGTM #19577, or maybe I wasn't the right primary reviewer), but they're correctible.

---

docs/RFCS/README.md, line 55 at r1 (raw file):

Previously, danhhz (Daniel Harrison) wrote…

Partitioning, too

I don't think we've had any external comments from the forum FCP post, though.

---

Comments from Reviewable

[rytaft]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

To the extent that we have observed an "unwritten rule" of a week before starting the FCP, I think that has been entirely by accident.

(Relatedly, I think this was also part of the motivation of the FCP.)

Yeah, this is what the FCP was supposed to be for, to ensure that there was a reasonable time for comments to come in. Maybe that time isn't long enough, but we should be wary of making this too heavyweight. A 9-day minimum seems too long to me.

With a shorter process, we won't be able to get feedback from everyone, and that's OK. The RFC author and a suitable primary reviewer should be able to agree and move on to implementation within a few days. We'll make mistakes (I was too quick to LGTM #19577, or maybe I wasn't the right primary reviewer), but they're correctible.

Makes sense - should we change the wording here at all, then? In the case of PR #19577, the FCP did work as intended... @knz, do you think we need to make the FCP longer? Or do you suggest some other process change?

---

Comments from Reviewable

[knz]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, rytaft wrote…

Makes sense - should we change the wording here at all, then? In the case of PR #19577, the FCP did work as intended... @knz, do you think we need to make the FCP longer? Or do you suggest some other process change?

So basically in general I'm good with a 1-week turnaround including FCP on "small" RFCs. But here the particular RFC that motivated this discussion/change was not a "small" RFC by any mean.

Let's be concrete here. Together with Nate we have developed a set of specific criteria to label the "complexity" of a project. This set is defined/described here: https://www.cockroachlabs.com/docs/stable/contribute-to-cockroachdb.html#how-complex-is-your-project

I want to propose the following:

• RFCs for small complexity projects can have no minimum delay
• RFCs for higher complexity projects must have at least a week of comments and a week of FCP (or alternatively, a minimum of 2 weeks cumulative discussion+FCP). Again in the interest of colleagues who take 2 weeks vacation OOO at a time.
• RFCs for medium complexity projects must have at least a week of cumulative discussion+FCP.

By this criteria, the SFU RFC had initially a 1 week minimum turnaround, then became 2 weeks once the discussion revealed there were non-trivial aspects.

How does this sound?

---

Comments from Reviewable

[rytaft]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, knz (kena) wrote…

So basically in general I'm good with a 1-week turnaround including FCP on "small" RFCs. But here the particular RFC that motivated this discussion/change was not a "small" RFC by any mean.

Let's be concrete here. Together with Nate we have developed a set of specific criteria to label the "complexity" of a project. This set is defined/described here: https://www.cockroachlabs.com/docs/stable/contribute-to-cockroachdb.html#how-complex-is-your-project

I want to propose the following:

• RFCs for small complexity projects can have no minimum delay
• RFCs for higher complexity projects must have at least a week of comments and a week of FCP (or alternatively, a minimum of 2 weeks cumulative discussion+FCP). Again in the interest of colleagues who take 2 weeks vacation OOO at a time.
• RFCs for medium complexity projects must have at least a week of cumulative discussion+FCP.

By this criteria, the SFU RFC had initially a 1 week minimum turnaround, then became 2 weeks once the discussion revealed there were non-trivial aspects.

How does this sound?

@knz - so are you proposing eliminating the FCP for small complexity RFCs? Or keep the 2-day FCP but no additional delay?

Otherwise all sounds good to me. Anyone disagree? @danhhz, @bdarnell?

---

Comments from Reviewable

[knz]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, rytaft wrote…

@knz - so are you proposing eliminating the FCP for small complexity RFCs? Or keep the 2-day FCP but no additional delay?

Otherwise all sounds good to me. Anyone disagree? @danhhz, @bdarnell?

for the small complexity thing I truly have no opinion, although 2-day seems reasonable.

---

Comments from Reviewable

[danhhz]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, knz (kena) wrote…

for the small complexity thing I truly have no opinion, although 2-day seems reasonable.

sgtm, thanks very much! I think FCP should be kept at 2 days for small rfcs, but no minimum delay once you've gotten an lgtm

---

Comments from Reviewable

[rytaft]

I've updated the commit message and the text based on the suggestion to require one week of PR review for medium complexity projects, and two weeks for high complexity projects.

---

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, danhhz (Daniel Harrison) wrote…

sgtm, thanks very much! I think FCP should be kept at 2 days for small rfcs, but no minimum delay once you've gotten an lgtm

Great - I've updated the text based on this discussion.

---

Comments from Reviewable

[knz]

---

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[bdarnell]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

But here the particular RFC that motivated this discussion/change was not a "small" RFC by any mean.

This is the crux of the issue - I believed that this was a small RFC and it wasn't until comments started arriving in the FCP that we realized the additional subtleties involved. The problem is that a medium-sized project was misjudged to be a small, so adding a policy that medium-sized projects get at least a week of discussion time wouldn't change anything in #19577 (at least for my part).

We could try to address this by adding a more rigorous process for sizing a project, but I'm not sure there's a cure that's better than the disease (we had a quorum of founders in favor of moving quickly in this case). I still believe it's best to allow things to move quickly even if that means occasionally discovering problems later than to tie things up with a more burdensome review process.

RFCs for higher complexity projects must have at least a week of comments and a week of FCP (or alternatively, a minimum of 2 weeks cumulative discussion+FCP). Again in the interest of colleagues who take 2 weeks vacation OOO at a time.

This is excessive. We don't need to structure things so that everyone can comment on every RFC regardless of vacation schedules. On a case-by-case basis we may want to require a particular person's review on an RFC, which may mean waiting for that person to come back from vacation, but I don't think such a long minimum duration is a good idea.

For complex RFCs, weeks-long discussions will happen naturally. But when it's not happening on its own I don't think we need to force it by slowing down the process. (I'd rather see a policy like "large projects should have at least two designated reviewers" instead of something time-based)

---

Comments from Reviewable

[rytaft]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

But here the particular RFC that motivated this discussion/change was not a "small" RFC by any mean.

This is the crux of the issue - I believed that this was a small RFC and it wasn't until comments started arriving in the FCP that we realized the additional subtleties involved. The problem is that a medium-sized project was misjudged to be a small, so adding a policy that medium-sized projects get at least a week of discussion time wouldn't change anything in #19577 (at least for my part).

We could try to address this by adding a more rigorous process for sizing a project, but I'm not sure there's a cure that's better than the disease (we had a quorum of founders in favor of moving quickly in this case). I still believe it's best to allow things to move quickly even if that means occasionally discovering problems later than to tie things up with a more burdensome review process.

RFCs for higher complexity projects must have at least a week of comments and a week of FCP (or alternatively, a minimum of 2 weeks cumulative discussion+FCP). Again in the interest of colleagues who take 2 weeks vacation OOO at a time.

This is excessive. We don't need to structure things so that everyone can comment on every RFC regardless of vacation schedules. On a case-by-case basis we may want to require a particular person's review on an RFC, which may mean waiting for that person to come back from vacation, but I don't think such a long minimum duration is a good idea.

For complex RFCs, weeks-long discussions will happen naturally. But when it's not happening on its own I don't think we need to force it by slowing down the process. (I'd rather see a policy like "large projects should have at least two designated reviewers" instead of something time-based)

Oops looks like I merged too soon... sorry about that. I can open another PR once we agree on the policy....

---

Comments from Reviewable

[knz]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, rytaft wrote…

Oops looks like I merged too soon... sorry about that. I can open another PR once we agree on the policy....

Maybe create an issue about the RFC, paste ben's comments in it, and either assign it to you (if you want / plan to continuing the iteration) or ask vivek to assign the issue to someone else.

---

Comments from Reviewable

[rytaft]

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

docs/RFCS/README.md, line 51 at r1 (raw file):

Previously, knz (kena) wrote…

Maybe create an issue about the RFC, paste ben's comments in it, and either assign it to you (if you want / plan to continuing the iteration) or ask vivek to assign the issue to someone else.

Good idea - I just created #20105.

---

Comments from Reviewable