[proposal] Improvements to the AWS CLI Contribution Process

[kdaily]

The proposal specifies improvements to the community contribution process for the AWS CLI. For a Markdown rendered version of this proposal, view it here.

⚠️ Important note for community members

The AWS CLI development team is trying out a new process where proposals for new features are opened and reviewed as pull requests on GitHub. This allows us to be more open on our thought process and solicit design feedback from community members. Currently, we do not recommend community members from opening pull requests with new proposals until we have fully solidified and documented the proposal process. However, we do recommend and encourage community engagement by:

• Reading the proposal and providing feedback. If you are in general agreement with the proposal, please add the 👍 reaction to this top-level comment. If you have any questions or suggestions, feel free to leave a comment in the pull request.

[codecov-commenter]

Codecov Report

Merging #6828 (b2eb4ff) into v2 (a4a014f) will decrease coverage by 30.99%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##               v2    #6828       +/-   ##
===========================================
- Coverage   93.68%   62.69%   -31.00%     
===========================================
  Files         350      350               
  Lines       35672    35679        +7     
  Branches     5105     5121       +16     
===========================================
- Hits        33421    22369    -11052     
- Misses       1646    12891    +11245     
+ Partials      605      419      -186     

Impacted Files	Coverage Δ
awscli/botocore/args.py	0.00% <0.00%> (-100.00%)	⬇️
awscli/botocore/config.py	0.00% <0.00%> (-100.00%)	⬇️
awscli/botocore/discovery.py	0.00% <0.00%> (-100.00%)	⬇️
awscli/botocore/docs/client.py	0.00% <0.00%> (-100.00%)	⬇️
awscli/botocore/docs/waiter.py	0.00% <0.00%> (-100.00%)	⬇️
awscli/botocore/docs/service.py	0.00% <0.00%> (-100.00%)	⬇️
awscli/botocore/errorfactory.py	0.00% <0.00%> (-100.00%)	⬇️
awscli/botocore/docs/paginator.py	0.00% <0.00%> (-100.00%)	⬇️
awscli/botocore/retries/throttling.py	0.00% <0.00%> (-100.00%)	⬇️
awscli/botocore/docs/bcdoc/__init__.py	0.00% <0.00%> (-100.00%)	⬇️
... and 60 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a4a014f...b2eb4ff. Read the comment docs.

[bugfood]

@tim-finnigan asked me for feedback in #5425. Overall, this seems like a good approach, given the current situation. Here are some specific critiques:

1. This seems to be a rather heavyweight process. While each element probably has a reason to exist, and I would find it hard to argue against any specific element, I would suggest being open to trimming out the less-important parts, perhaps in the future once the policy gets some exercise and the people impacted by it can give specific feedback.
2. The policy specifically mentions feature requests and bugs; the policy treats them distinctly. What about cleanups? This would include e.g. removal of unused code, readability improvements, and refactors. I think cleanups should be treated in the same way as bugs, even though they don't address any user-impacting behavior. Cleanups improve the developer experience and can be a good way for a new contributor to begin to become familiar with the codebase and the contribution process.
3. Be aware that the requirement for 10 upvotes on feature requests is easily gamed, especially for corporate contributors: just ask colleagues to upvote.
4. I am interested in the PR acceptance criteria (mentioned but not yet provided). In particular, I think this should be geared toward requiring contributors to leave the codebase in a state that is not worse than before, without requiring contributors to make major changes outside the initial scope of the PR unless absolutely necessary.

[kdaily]

@bugfood,

Thank you for providing your feedback! I think you bring up some good points, but hopefully I can clarify.

1. This seems to be a rather heavyweight process. While each element probably has a reason to exist, and I would find it hard to argue against any specific element, I would suggest being open to trimming out the less-important parts, perhaps in the future once the policy gets some exercise and the people impacted by it can give specific feedback.

There is intention behind the friction in the process, but I think some of it can be alleviated. We have to acknowledge that the AWS CLI is used extensively in production systems across the world, and thousands of users depend on it's stability. So, we need to move carefully and methodically to make sure we maintain that stability. The intention is to set up guard rails to make contributions successful.

Additionally, I would not expect that a new user (or any user for that matter) should need to read our contributing guide from cover-to-cover to be successful. We're planning on writing a "Getting Started" guide that summarizes what is needed to contribute, and it will somewhat be up to us as maintainers to guide users through next steps. We've also planned regular retrospective meetings to evaluate the process and make adjustments - if we find that steps are onerous or unnecessary, we may trim them.

2. The policy specifically mentions feature requests and bugs; the policy treats them distinctly. What about cleanups? This would include e.g. removal of unused code, readability improvements, and refactors. I think cleanups should be treated in the same way as bugs, even though they don't address any user-impacting behavior. Cleanups improve the developer experience and can be a good way for a new contributor to begin to become familiar with the codebase and the contribution process.

Good suggestion! We only specified a few types of changes that would not be taken and didn't enumerate all of the ones that would, but a cleanup might be one of those that would. A few thoughts on that:

1. Our current SDK-wide label strategy uses three categories - bug, feature-request, and guidance (which is now moved to GitHub Discussions). We have repository-specific 'enhancer' labels, so I would classify a cleanup as a feature-request plus an enhancement. I've considering other intent-based labels, but we don't have anything completely specified yet. You can see a draft of our existing labels and potential new ones here.
2. Some cleanups might be difficult because of the relationship of the AWS CLI to botocore and need more care and coordination, but you could also argue that can be seen as an exercise to improve the documentation. Refactors would probably fall into this; if a feature request is big enough to require a refactor, we would probably use a further open design discussion (via an RFC, like this one), to drive that. We discuss that as future work here.
3. Some would be specifically excluded from community contribution, like changes to dependencies. These are enumerated (not exhaustively) here.
4. We will enable significant automation for linting, formatting, etc. that should obviate the need for massive cleanup efforts of those types.

3. Be aware that the requirement for 10 upvotes on feature requests is easily gamed, especially for corporate contributors: just ask colleagues to upvote.

We're considerate of that. We are currently using upvotes in some capacity now and haven't experienced that. We can also look at how much constructive commenting activity there is to supplement pile-on 👍🏻 or "+1" comments, but that's less automated at the moment.

4. I am interested in the PR acceptance criteria (mentioned but not yet provided). In particular, I think this should be geared toward requiring contributors to leave the codebase in a state that is not worse than before, without requiring contributors to make major changes outside the initial scope of the PR unless absolutely necessary.

I agree with you there! We're hoping the heavy lifting design discussions happen in the issue ahead of time, and the PR is focused on the implementation code. I think your suggested sentiment is captured in PR acceptance criteria listed here in the pre-review stage and the Review stage.

Hope this answers your questions or concerns!

[kyleknap]

Looks good. Just had one comment based on how we adjusted to using the new GitHub projects. Otherwise, feel free to add a new commit that moves the proposal to Accepted similar to the source distribution proposal: 647b0e1

[kyleknap]

I wonder if we want to update the link here to the new GitHub projects feature we are using instead of the GitHub project board: https://docs.github.com/en/issues/trying-out-the-new-projects-experience/about-projects

[kyleknap]

Looks good! 🚢

[lorengordon]

@kdaily Could this be merged to the develop branch also, so it is easier to find when browsing the repo? Or at least a link from the CONTRIBUTING and README docs on the develop branch?