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.

Sign up for GitHub

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

Jump to bottom

RFC 53: updates to RFC process #336

Merged
merged 15 commits into from
Jul 7, 2021
Merged

RFC 53: updates to RFC process #336

merged 15 commits into from
Jul 7, 2021

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Jun 13, 2021 •
edited
Loading

As a follow up to a retrospective the CDK team has performed about the RFC process.

This PR proposes the following changes:

  • Decouple the creation of the tracking issue from the submission of the RFC pull request to simplify the initial phase.
  • Add a concept of "API Bar Raiser" who needs to approve the "working backwards" section of the RFC
  • Remove RFC PR # from document - the tracking issue is the RFC number.
  • Simplify template a little
  • Add details in the main README which describes each step
  • Additional minor changes

See https://github.com/cdklabs/team-internal/pull/18 for internal process changes.

Elad Ben-Israel added 4 commits June 13, 2021 12:42
Updates to RFC process
4e783e3 
As a follow up to a retrospective the CDK team has performed about the RFC process. This PR proposes the following changes:

[ADD]
Update 0000-template.md
78499a4
Soem template updates
4f3f3ed
readme updates
9b48abe
@eladb eladb changed the title DRAFT (DO NOT REVIEW YET): Updates to RFC process Updates to RFC process Jun 13, 2021
@eladb eladb marked this pull request as ready for review June 13, 2021 12:21
nija-at
nija-at reviewed Jun 15, 2021
View reviewed changes
.github/ISSUE_TEMPLATE/tracking-issue.md Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/tracking-issue.md Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/tracking-issue.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Elad Ben-Israel and others added 2 commits June 18, 2021 13:02
Update 0000-template.md
6acf4b5 
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
Update README.md
a775fa2 
Co-authored-by: Niranjan Jayakar <nija@amazon.com>
@eladb eladb changed the title Updates to RFC process RFC 53: updates to RFC process Jun 22, 2021
@eladb
Copy link
Contributor Author

eladb commented Jun 22, 2021

From our conversation amongst CDK tech leads: we will reframe the RFC process to use an “API Bar Raiser” model instead of an approver model. Every RFC will get assigned a “tier” and bar raiser for API Bar Raising by the CDK Tech Leadership (email aws-cdk-tech-leads@).

  • T1 - Launches (L7)
  • T2 - Core AWS CDK APIs, incl. JSII CLIs (L6+)
  • T3 - AWS Construct Library APIs (L5+)

The alias of the bar raiser will be included in the RFC document itself and the same bar raiser will be required if major API changes are performed.

As for the rest of the RFC, the RFC PR will undergo the normal PR process of the CDK team, which means that an approver from the core team will eventually need to approve the PR in order to merge it. The API bar raiser will need to sign-off on the “working backwards” section, which primarily represents the public API of the feature.

BenChaimberg
BenChaimberg reviewed Jun 24, 2021
View reviewed changes
README.md Outdated Show resolved Hide resolved
nija-at
nija-at reviewed Jun 29, 2021
View reviewed changes
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/tracking-issue.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
eladb
eladb commented Jun 29, 2021
View reviewed changes
README.md Outdated Show resolved Hide resolved
Updates
1d7b560 
* Change RFC title not to include issue
* Rephrase FAQ related to design
* Multiple updates in README
* Simplify PR template
* Minor updates to tracking issue template
@eladb eladb requested a review from BenChaimberg June 30, 2021 12:47
@eladb eladb added the status/final-comment-period Pending final approval label Jun 30, 2021
RomainMuller
RomainMuller reviewed Jun 30, 2021
View reviewed changes
.github/ISSUE_TEMPLATE/tracking-issue.md Outdated Show resolved Hide resolved
0000-template.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Elad Ben-Israel and others added 2 commits June 30, 2021 17:21
@RomainMuller
Update .github/ISSUE_TEMPLATE/tracking-issue.md
919ae46 
Co-authored-by: Romain Marcadier <rmuller@amazon.com>
@RomainMuller
Update 0000-template.md
e9bae64 
Co-authored-by: Romain Marcadier <rmuller@amazon.com>
skinny85
skinny85 reviewed Jul 3, 2021
View reviewed changes
Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

Looks good! I think the new "StackSets support" RFC might be a good candidate to try out this new process on.

I guess the only irritating part is that the template is changing, so the author might have to do some work to move to the new template...

@eladb
Copy link
Contributor Author

eladb commented Jul 4, 2021

Looks good! I think the new "StackSets support" RFC might be a good candidate to try out this new process on.

Yes, let's apply this scheme to this RFC!

@eladb eladb merged commit 5cc47c4 into master Jul 7, 2021
@eladb eladb deleted the benisrae/rfc-2 branch July 7, 2021 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RomainMuller RomainMuller RomainMuller approved these changes

@njlynch njlynch Awaiting requested review from njlynch

@rix0rrr rix0rrr Awaiting requested review from rix0rrr

@BenChaimberg BenChaimberg Awaiting requested review from BenChaimberg

@skinny85 skinny85 Awaiting requested review from skinny85

@nija-at nija-at Awaiting requested review from nija-at

Assignees
No one assigned
Labels
status/final-comment-period Pending final approval
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eladb @RomainMuller @skinny85 @BenChaimberg @nija-at