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

chore: simplify RFC format #222

Merged
merged 5 commits into from
Oct 8, 2020
Merged

chore: simplify RFC format #222

merged 5 commits into from
Oct 8, 2020

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Aug 9, 2020

A proposal to reduce the complexity of RFCs by simplifying the template to a PR/FAQ format.

Rendered version

A proposal to reduce the complexity of RFCs by simplifying the template to a PR/FAQ format.
@eladb eladb requested a review from a team August 9, 2020 06:38
@eladb eladb changed the title chore: new minimal RFC template chore: simplify RFC template Aug 9, 2020
@eladb eladb changed the title chore: simplify RFC template chore: simplify RFC format Aug 9, 2020
@mergify
Copy link
Contributor

mergify bot commented Aug 9, 2020

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

0000-template.md Outdated Show resolved Hide resolved
0000-template.md Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
nija-at
nija-at previously requested changes Sep 3, 2020
0000-template.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
0000-template.md Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
>
> - [Graphviz](http://graphviz.it/#/gallery/structs.gv)
> - [PlantText](https://www.planttext.com)
> If this is a major feature (~6 months of work), write the PRESS RELEASE which
Copy link
Contributor

Choose a reason for hiding this comment

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

This is covered in Working Backwards above. We should merge these paragraphs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the working backwards section. Not sure I follow...

@ericzbeard
Copy link
Contributor

How is this a simplification? It's hard to tell from the diff how this makes an RFC easier/faster to write.

@nija-at
Copy link
Contributor

nija-at commented Sep 4, 2020

How is this a simplification? It's hard to tell from the diff how this makes an RFC easier/faster to write.

@ericzbeard - how do you propose this be made visible? The nature of applying unified diff on markdown docs does not render for nice diff'ing.

I've updated the commit description with a link to the rendered version. You should be able to side by side compare the old rendered version with the new one to see if this is simplified.

Copy link
Contributor

@NetaNir NetaNir left a comment

Choose a reason for hiding this comment

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

I like it, I think it makes the RFC process more of a tool to help the writer frame their thoughts than a chore.
One general comment, the addition of the Press release creates redundancy with others section, for example: CHANGELOG and "What are we launching today" + "Is this a breaking change". I would suggest dropping the README section and the CHANGELOG section in favor of the press release.


# Design Summary
### README
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need the README section in addition to the content of the PRESS RELEASE and Working Backwards, I feel like there will be a lot of duplicate between the two.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See line 12, the requirement is to include "one or more artifacts", not all of them.

Co-authored-by: Niranjan Jayakar <nija@amazon.com>
@mergify mergify bot dismissed nija-at’s stale review October 4, 2020 15:40

Pull request has been modified.

@nija-at nija-at added the pr/do-not-merge Let mergify know not to auto merge label Oct 5, 2020
nija-at
nija-at previously approved these changes Oct 5, 2020
Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

LGTM.

Added 'do-not-merge' to give others a chance to take a look.

0000-template.md Outdated Show resolved Hide resolved
0000-template.md Show resolved Hide resolved
0000-template.md Show resolved Hide resolved
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
@mergify mergify bot dismissed nija-at’s stale review October 5, 2020 09:44

Pull request has been modified.

@eladb eladb added the status/final-comment-period Pending final approval label Oct 5, 2020
@eladb eladb merged commit f4a0608 into master Oct 8, 2020
@eladb eladb deleted the new-rfc-template branch October 8, 2020 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/do-not-merge Let mergify know not to auto merge status/final-comment-period Pending final approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants