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

Unclear sentence under "Design" section in CONTRIBUTION.md #22601

Closed
azatoth opened this issue Oct 21, 2022 · 3 comments
Closed

Unclear sentence under "Design" section in CONTRIBUTION.md #22601

azatoth opened this issue Oct 21, 2022 · 3 comments
Labels
bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p1

Comments

@azatoth
Copy link
Contributor

azatoth commented Oct 21, 2022

Describe the issue

Currently, for the section "Step 2: Design" (also marked as "optional" in the TOC);
It contains the following paragraphs:

In some cases, it is useful to seek feedback by iterating on a design document. This is useful when you plan a big change or feature, or you want advice on what would be the best path forward.

In many cases, the GitHub issue is sufficient for such discussions, and can be sufficient to get clarity on what you plan to do. If the changes are significant or intrusive to the existing CDK experience, and especially for a brand new L2 construct implementation, please write an RFC in our RFC repository before jumping into the code base. L2 construct implementation pull requests will not be reviewed without linking an approved RFC.

The main issue I see is that the last sentence "L2 construct implementation pull requests will not be reviewed without linking an approved RFC" are:

  1. It contradict the rest of the sections, i.e. it makes it a strict requirement instead of a possible suggestion.
  2. If we are referring to that RFCs must already exists, then it makes it implausible to create a PR for an already existing L2 construct if it doesn't already have an RFC.
  3. If we are meaning that you need to create an RFC for any implementation and have it approved first, then it stifles any community participation, slows everything down to a halt, and makes it a bureaucratic hell.

I would suggest to drop this language altogether.

Links

https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md#step-2-design

@azatoth azatoth added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Oct 21, 2022
@peterwoodworth
Copy link
Contributor

Thanks for letting us know about this section @azatoth, I agree that it's unclear and misleading.

I'm not sure that the RFC rule is strict about needing a new RFC for every individual new L2 construct. That could be obstructive more than constructive in some cases, like for each new IOT action we can add and other similar constructs. I'd imagine that at the very least for this case we would only need to approve an RFC for an arbitrary action rather than an RFC for every individual action.

Looking at these actions, it doesn't seem there was an RFC at all but rather discussion went on in the initial reported issue with a team member. So I wouldn't be worried about trying to contribute more actions in this case.

We will figure out how to make our requirements much more clear soon and then update the guide 🙂

@peterwoodworth peterwoodworth added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Oct 24, 2022
@peterwoodworth peterwoodworth added the bug This issue is a bug. label May 2, 2023
@peterwoodworth peterwoodworth removed their assignment Oct 16, 2023
@peterwoodworth
Copy link
Contributor

we'll be updating this soon. Closing now so that this issue doesn't linger unnecessarily, thanks for reporting

@peterwoodworth peterwoodworth closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2023
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

No branches or pull requests

2 participants