-
Notifications
You must be signed in to change notification settings - Fork 629
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
docs: update stabilization section #4751
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4751 +/- ##
=======================================
Coverage 91.88% 91.88%
=======================================
Files 485 485
Lines 41292 41292
Branches 5317 5317
=======================================
Hits 37943 37943
Misses 3292 3292
Partials 57 57 ☔ View full report in Codecov by Sentry. |
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 rewrote this, using your points, with the aim of improving the flow and clarity of the instructions. Please let me know if you have any issues. But now, to me, LGTM.
at stabilization. | ||
1. Allow 1 month for 2 other maintainers to review the package and handle any | ||
feedback from the community. | ||
1. If there are no remaining issues, publish version 1.0.0. If there are |
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 think we should avoid canceling the stabilization of a package. Canceling the stabilization means it'll never happen. Instead, we can extend the waiting period with the extended waiting period being defined on a case-by-case basis, based on the severity of the issues.
README.md
Outdated
1. No open issues or pull requests that might lead to breaking changes. For | ||
example, issues that suggest new non-breaking features are fine to exist | ||
at stabilization. | ||
1. Allow 1 month for 2 other maintainers to review the package and handle any |
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 think you forgot to note that 2 other maintainers must review the package, so I added it in.
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 think no feedback 1 month automatically means the approval.
I also don't want to assign core team people randomly to review tasks.
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.
No feedback after 1 month could also mean no one has looked at the package. I think some degree of approval from the core team should be a requirement, even if it's one other person.
package must go through the following steps to achieve stabilization: | ||
|
||
1. Publish version 1.0.0-rc.1 once meeting the following requirements: | ||
1. Approved by at least 2 maintainers. There must be consensus that the |
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 repeat what there must be a consensus on in the step below for emphasis.
Landing for now. Let's keep iterating on details. |
I think the current wording allows for reviews from the internal team. LGTM. |
closes #4661