-
Notifications
You must be signed in to change notification settings - Fork 129
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
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌