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.

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

docs: document RFC final comment period guidelines #10181

Merged
merged 1 commit into from Oct 24, 2016
Merged

docs: document RFC final comment period guidelines #10181

merged 1 commit into from Oct 24, 2016

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Oct 24, 2016

This change is Reviewable

@tbg
Copy link
Member

tbg commented Oct 24, 2016

LGTM

@a-robinson
Copy link
Contributor

:lgtm:


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


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

   and if there is consensus to proceed with the project, announce that
   the RFC is entering its Final Comment Period (FCP) using the method
   de jour (https://forum.cockroachlabs.com/ at the time of this

s/de/du :)


Comments from Reviewable

Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

and if there is consensus to proceed with the project, announce that
the RFC is entering its Final Comment Period (FCP) using the method
de jour (https://forum.cockroachlabs.com/ at the time of this
writing). The FCP should last a minimum of two working days to allow
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The FCP should also be announced on the github PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, with a bit of a rewrite.

consensus to proceed after the FCP, change the `Status` field of the
document to `in-progress`, update the `RFC PR` field, and merge the
PR. If the project is rejected, either abandon the PR or merge it
with a status of `rejected` (depending on whether the document and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth mentioning that accepted RFCs can hit snags during implementation that could have been caught during the RFC review process. This is undesirable and something we should try to avoid, but hard to eliminate completely unless we make the RFC process overly onerous. For example, what happens if an expert on part of the system is on an extended vacation when an RFC is being reviewed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would be the purpose of such a mention? In other words, what is the guidance being issued to the reader, when they encounter such a case?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose would be to set expectations: an accepted RFC is not an immutable contract. We can back out of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, PTAL.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tamird
Copy link
Contributor Author

tamird commented Oct 24, 2016

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


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

Previously, a-robinson (Alex Robinson) wrote…

s/de/du :)

Done, thanks!

Comments from Reviewable

@mrtracy
Copy link
Contributor

mrtracy commented Oct 24, 2016

:lgtm:


Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.


Comments from Reviewable

@tamird tamird merged commit 4c35c5d into cockroachdb:master Oct 24, 2016
@tamird tamird deleted the rfc-fcp branch October 24, 2016 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants