-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
doc: update deprecations info in COLLABORATOR_GUIDE #20523
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 it is good to keep it as as. This makes it clear to the reader that we try to report everything we find but we can not always handle everything. It also helps collaborators to know that this is something they should do when opening a PR.
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.
This is the paragraph most in need of removal IMO.
If we're trying to communicate that to the community, then the
COLLABORATOR_GUIDE.md
is the wrong place to do it. Information aimed at end users should be indeprecations.md
instead.Perhaps more importantly, I take issue with the idea that this paragraph makes anything clear. It is almost exclusively weasel words. The very first sentence starts with: "A best effort will be made..." What does "best effort" mean in this context? (Answer: It is meaningless. It seems significant but actually indicates nothing specific. Does that mean the Node.js Twitter will tweet out the information, for example? Maybe. No one can say. It could mean anything. So it means nothing.)
The paragraph appears to contradict itself on timing too. At the very least, the juxtaposition of "as soon as possible" with "preferably" muddies things. (Again, it all sounds like it's saying something significant but is saying nothing specific or meaningful IMO.)
If deprecations need to be disclosed to the community via one or more specific channels that Collaborators need to be aware of and responsible for, all within certain timeframes, then let's put that information in the doc. But that information does not exist and will take time to hash out. As it stands, we have a paragraph of meaningless platitudes directed at people who aren't even going to read this doc but are likely to read a different doc. Let's remove it.
That "something they should do" is the only specific information in the paragraph and it's why I added this sentence to the doc, which is better than the existing text IMO because it clearly explains how to do the thing you're supposed to do:
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.
For me the first sentence seems pretty important. I understand the
something should be done
part as a request to check CITGM and to open PRs in userland modules to fix the upcoming issues. This is done with a "best effort" and we can not guarantee anything and decide if it makes sense to do it or not.