Docs: Clarify when an RFC can be merged #8
Conversation
|
Just noticed I (non-TSC member) also have the access to merge, should the access be recalled? |
README.md
Outdated
|
|
||
| An RFC may be merged by a TSC member provided that: | ||
|
|
||
| 1. Every TSC member who wishes to comment on the RFC has done so. |
There was a problem hiding this comment.
I'm not sure how to evaluate this condition in practice. For example, if I want to merge an RFC, how would I determine if everyone who wishes to comment has done so? It seems like it would be unclear whether (a) someone hasn't commented because they're busy but plans to comment at some point, or (b) someone doesn't wish to comment.
There was a problem hiding this comment.
Yeah, that does seem fuzzy in retrospect. Maybe this should be replaced with some time period? Like an RFC must have an initial comment period of n weeks, which would give everyone enough time to comment?
There was a problem hiding this comment.
My experience with #3 was:
- Around the time the PR was opened, I made some comments but didn't have a strong opinion either way on whether it should be merged.
- Later, a few team members discussed certain aspects of the RFC and I think some changes were made. I didn't take the time to understand these discussions while they were going on (since it seemed to be changing somewhat quickly)
- I would have liked to have a chance to look at the final version after all changes were made, just to double-check that nothing was added that I would have a strong objection to. However, I wasn't sure if more changes were forthcoming (since I hadn't been following the discussions closely), so I put off looking it over. After a bit of time, the PR was merged and I hadn't ended up looking it over. Of course, given the number of approvals and the time elapsed, it's understandable that the PR was merged, since I hadn't mentioned I wanted to review the final version. I think the main issue in this case was that the PR had changed and I wasn't sure if it was about to change again (without reading through all the discussions).
Having a comment period is a good idea. I would slightly prefer a final comment period (with a length of a week, say) rather than an initial comment period. Entering the final comment period would imply that the PR has sustained some feedback, and isn't expected to need further changes barring a major objection.
If the design is changed as a result of objections during the final comment period, the timer would be reset to make sure everyone is still okay with those changes. (This does have a downside of potentially making the process take a bit longer.)
There was a problem hiding this comment.
Could we ping the TSC with a set amount of time to respond when the RFC is in a complete state and any concerns that have been brought up have been addressed?
There was a problem hiding this comment.
I know we're trying to avoid pinging teams in GitHub, but because it seems unlikely there will be many concurrent RFCs going on, it feels to me like it could be a good use of that feature.
|
@aladdin-add thanks for noticing that, I've adjusted the settings. |
README.md
Outdated
|
|
||
| 1. Every TSC member who wishes to comment on the RFC has done so. | ||
| 1. All outstanding concerns and comments have been addressed. | ||
| 1. Consensus has been reached on merging the RFC (no TSC member dissents to merging). This may be done on the RFC pull request by TSC members marking the pull request as approved. |
There was a problem hiding this comment.
To clarify, does this also require that a quorum of TSC members have approved the PR (in addition to having no dissenters)?
There was a problem hiding this comment.
Yes, I'd envision this as the same process we follow for core changes.
README.md
Outdated
|
|
||
| An RFC may be merged by a TSC member provided that: | ||
|
|
||
| 1. Every TSC member who wishes to comment on the RFC has done so. |
There was a problem hiding this comment.
My experience with #3 was:
- Around the time the PR was opened, I made some comments but didn't have a strong opinion either way on whether it should be merged.
- Later, a few team members discussed certain aspects of the RFC and I think some changes were made. I didn't take the time to understand these discussions while they were going on (since it seemed to be changing somewhat quickly)
- I would have liked to have a chance to look at the final version after all changes were made, just to double-check that nothing was added that I would have a strong objection to. However, I wasn't sure if more changes were forthcoming (since I hadn't been following the discussions closely), so I put off looking it over. After a bit of time, the PR was merged and I hadn't ended up looking it over. Of course, given the number of approvals and the time elapsed, it's understandable that the PR was merged, since I hadn't mentioned I wanted to review the final version. I think the main issue in this case was that the PR had changed and I wasn't sure if it was about to change again (without reading through all the discussions).
Having a comment period is a good idea. I would slightly prefer a final comment period (with a length of a week, say) rather than an initial comment period. Entering the final comment period would imply that the PR has sustained some feedback, and isn't expected to need further changes barring a major objection.
If the design is changed as a result of objections during the final comment period, the timer would be reset to make sure everyone is still okay with those changes. (This does have a downside of potentially making the process take a bit longer.)
|
I don't think extending the time it takes to approve an RFC should be an issue. The point of the RFC process is really to slow things down and ensure we are considering the value of a significant change vs. the amount of work and potential downsides. I'm going to update this page with a suggestion for how to do initial/final commenting periods. |
|
Okay, I've updated a proposal with the concept of an initial comment period and a final comment period. Let me know what you think. |
There was a problem hiding this comment.
I have one question, but otherwise this LGTM.
This is more of an implementation detail, but I would love to be pinged on the issue when the final commenting period begins (maybe we could ping @eslint/eslint-tsc?). Seems like this could all be automated as well (if it isn't already).
|
Yeah, I think it makes sense to ping the team when the final commenting period has started. We can probably automate that, too, with the bot. |
README.md
Outdated
| When an RFC is submitted, it goes through the following process: | ||
|
|
||
| 1. **Initial commenting period (21 days)** - the community and ESLint team are invited to provide feedback on the proposal. During this period, you should expect to update your RFC based on the feedback provided. Very few RFCs are ready for approval without edits, so this period is important for fine-tuning ideas and build consensus. (A PR in the initial commenting period has the **Initial Commenting** label applied.) The initial commenting period may be extended with a TSC vote if the RFC has been unable to reach consensus but the team believes it worthwhile to continue developing the RFC. The TSC may also decide to abandon an RFC that has failed to reach consensus after 21 days. | ||
| 1. **Final commenting period (7 days)** - when all feedback has been addressed, the pull request enters the final commenting period where ESLint TSC members provide their final feedback and either approve of the pull request or state their disagreement. (A PR in the final commenting period has the **Final Commenting** label applied.) ESLint TSC members are notified through GitHub when an RFC has passed into the final commenting period. |
There was a problem hiding this comment.
Who decides when a PR should enter final commenting period? Is it up to the PR author? (It's good that we're providing general criteria like "when all feedback has been addressed", but presumably someone would need to judge whether those criteria have been met for a particular RFC.)
I think leaving it up to the PR author would work reasonably well. Although an author could prematurely try to move their RFC to the final commenting period before feedback has been adequately addressed, this would likely result in objections during the final comment period. As a downside, RFC authors who are not on the TSC would probably need to notify a TSC member or trigger a bot to move into final commenting period, and they might not realize they need to take this additional step.
There was a problem hiding this comment.
I agree. I think allowing the PR author to request the final commenting period makes sense. My thoughts:
- I think we can trust TSC members to apply the "Final Commenting" label when they believe it's time.
- For non-TSC members, it seems like it should be good enough to have one TSC member review the RFC to ensure it's ready and apply the label.
(Trying to avoid a vote for every part of the process.)
|
I've updated the proposed process based on the latest round of feedback. I think we are getting close to consensus on this, so I'd appreciate everyone giving this another look. |
Not an RFC!
Just wanting to clarify when an RFC can be merged, as there were some questions about this.