Skip to content

first pass of RFC refactor #87

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

Merged
merged 3 commits into from
Apr 20, 2017
Merged

Conversation

joeyaiello
Copy link
Contributor

@joeyaiello joeyaiello commented Mar 31, 2017

As discussed by the @PowerShell/powershell-committee in our last community call, this is a first attempt at moving to a model where comments are reviewed in a pull request.

This process requires that we make a Review - Open for comments label.

Also, if it makes sense to anyone else, I want to write a "quick start guide" for writing an RFC that has a lot less formal language and is more step-by-step.


This change is Reviewable

@daxian-dbw
Copy link
Member

I want to write a "quick start guide" for writing an RFC that has a lot less formal language and is more step-by-step.

👍 Please do @joeyaiello, that will be helpful.

When the Pull Request is submitted, ensure `Allow edits from maintainers` is checked so that the Committee can add the RFC number to the draft and also update the title accordingly.
* When submitted, RFC documents shall follow the naming convention of `RFCNNNN-<Title>.md`.
* Authors of RFCs shall not assign the RFC number (leave the `NNNN` in the filename).
* When the Pull Request is submitted, the author shall select `Allow edits from maintainers` is checked so that the Committee can add the RFC number to the draft, update the title, and fix the filename.
Copy link
Member

Choose a reason for hiding this comment

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

Wording reads weird, I think it should be:

the author shall ensure `Allow edits from maintainers` is checked so ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, typo from two possible format choices. How's

...the author shall check Allow edits from maintainers so that...

Copy link
Member

Choose a reason for hiding this comment

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

@joeyaiello I'm ok with that proposed change

* New proposed drafts should be submitted as a Pull Request from the Author's fork into the `Draft-Accepted` folder.
* The Author shall ensure that the `Allow edits from maintainers` box is checked so that a Committee Member can assign the next RFC number.
* If the Pull Request has followed the correct template and process, a Committee Member will assign the label `Review - Open for comments` to the Pull Request to indicate that anyone can comment on the content of the submission.
Typically, one or two months is allowed for comments, though this may be extended if a submission is particularly conentious or hasn't received enough feedback for the Committee to feel comfortable making a decision.
Copy link
Member

Choose a reason for hiding this comment

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

typo: contentious

* The Author shall ensure that the `Allow edits from maintainers` box is checked so that a Committee Member can assign the next RFC number.
* If the Pull Request has followed the correct template and process, a Committee Member will assign the label `Review - Open for comments` to the Pull Request to indicate that anyone can comment on the content of the submission.
Typically, one or two months is allowed for comments, though this may be extended if a submission is particularly conentious or hasn't received enough feedback for the Committee to feel comfortable making a decision.
* When the Committee closes the comment period, the Author should update the RFC and Pull Request to address the comments.
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should have a commit explicitly called comments addressed or something to that effect for the PR

* If the Pull Request has followed the correct template and process, a Committee Member will assign the label `Review - Open for comments` to the Pull Request to indicate that anyone can comment on the content of the submission.
Typically, one or two months is allowed for comments, though this may be extended if a submission is particularly conentious or hasn't received enough feedback for the Committee to feel comfortable making a decision.
* When the Committee closes the comment period, the Author should update the RFC and Pull Request to address the comments.
* The Committee shall vote to accept or reject the RFC and accept/close the Pull Request accordingly.
Copy link
Member

Choose a reason for hiding this comment

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

replace accept with merge

Typically, one or two months is allowed for comments (see `Comments Due` above).
* New comments are not being sought.
* No one has begun implementing the RFC, and there are no current plans to implement the RFC.
* The community is invited to implement the RFC (unless explicitly stated in the RFC).

Choose a reason for hiding this comment

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

I think that the default assumption should be that the proposer is planning to implement the RFC. Otherwise, the RFC's can end up as a bunch of unfunded mandates. If you want to open the implementation up, that would fit in issues which there is a mechanism for tagging things that are up for grabs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't currently have that assumption (though we may need to built it in if RFCs become too high-volume...they're not there yet today, however), but I think an up-for-grabs label probably makes sense.

Copy link
Member

Choose a reason for hiding this comment

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

I like the up-for-grabs label, should add some text for it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PowerShell/powershell-committee agrees that we should add a key to the front-matter called Plan to implement: <Yes | No> and if not, we may be slow to respond/approve, and the RFC should be marked as Up-for-grabs once it's accepted.

Design and implementation is considered complete.
Any proposed changes would be through a new RFC.
RFCs in the `Final` state are considered fully complete and implemented in PowerShell.
Any proposed changes should be made through a new RFC or via an Issue in the [PowerShell/PowerShell repository](https://github.com/powershell/powershell).

Choose a reason for hiding this comment

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

Rather than an issue, why not a PR to modify the existing RFC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really want to get into the business of deciding what constitutes a "modification to an existing RFC" vs. "a fully new feature". It's a lot easier to just say "that's done now, next RFC".

I could be wrong here, though. Do you guys have experience with this on either side at Chef?

Choose a reason for hiding this comment

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

Yeah, for example, our RFC 062 - Exit Codes
Initial adoption of the RFC - chef-boneyard/chef-rfc#157
Modifications - chef-boneyard/chef-rfc#189
- chef-boneyard/chef-rfc#197
- chef-boneyard/chef-rfc#225

In large part it depends on how big of a change you are talking about. If you are substantially changing the behavior, I think a new RFC is warranted. If you are just extending existing behavior or clarifying the direction, a modification to the exiting RFC is sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed this with the @PowerShell/powershell-committee, and we agree that once something is accepted into the Final state, it should remain unchanged. In the long run, it makes things simpler for us. If necessary, the new RFC should reference the old. (Ultimately, I think it's a positive if RFCs are shorter and easier to parse.)

If this becomes a problem later, we can always change our approach here.

Choose a reason for hiding this comment

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

👌

@@ -3,7 +3,7 @@ RFC: RFC0000
Author: Steve Lee
Copy link
Member

Choose a reason for hiding this comment

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

You should add your name here


## RFC Template

RFC documents shall follow the following template:

```markdown
---
RFC: RFC<four digit unique incrementing number - assigned by Committee>
RFC: RFC<four digit unique incrementing number assigned by Committee, this should be left blank by the author>
Copy link
Member

Choose a reason for hiding this comment

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

change should to shall

@joeyaiello joeyaiello merged commit c9dde98 into PowerShell:master Apr 20, 2017
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.

5 participants