Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.3k
doc: clarify text about internal module changes#22024
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
Conversation
ghost commented Jul 30, 2018 • edited by ghost
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by ghost
Uh oh!
There was an error while loading. Please reload this page.
Trott left a comment • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
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.
"These are subject to change" is correct. The word subject is a verb here, not a noun. A more wordy way of saying it would be "These may be subjected to changes". However, probably for historical reasons, subject to change is the natural, idiomatic way to say it for native speakers.
(Aside: This is actually really interesting for me. I've never thought twice about the phrase subject to change and so I'm now realizing that I never really thought about exactly what each word means.)
lib/internal/readme.md Outdated
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.
subject to change is a phrase so this change is incorrect.
Trott commented Jul 30, 2018
If you want to clarify it for people unfamiliar with the subject to change idiom, how about changing from this:
...to this:
Seems both clearer and more succinct to me. (I'm also 👍 on eliminating the emphasis on any if you want to do that too.) |
Trott commented Jul 30, 2018
And I'd also be 👍 on making it clear what these refers to, so maybe this?:
|
richardlau commented Jul 30, 2018
I think this is one of those times that being a native speaker is a disadvantage when trying to explain things. You know whether a phrase is correct/incorrect but you're not exactly sure how to explain why. |
ghost commented Jul 30, 2018
@Trott & @richardlau: |
Trott commented Jul 30, 2018
The commit message is hard to understand. How about this?: doc: clarify text about internal module changes Simplify phrasing for clarity and succinctness. |
no objection to new change; holding off on approving until commit message is 👍
Simplify phrasing for clarity and succinctness.
ghost commented Jul 30, 2018
No problem. For a Node lover I hope I can make it better according to our suggestions and ideas. |
gireeshpunathil commented Jul 30, 2018
Looks like a thin boundary between clarity and expressiveness. Agreeing to all the comments above, and in addition acknowledging the doc improvement if it helps easy consumption for wider audience. |
Trott commented Jul 30, 2018
vsemozhetbyt commented Jul 30, 2018
Node.js Collaborators, please, add 👍 here if you approve fast-tracking. |
vsemozhetbyt commented Jul 30, 2018
@richardlau Do you still object to this change? |
vsemozhetbyt commented Jul 31, 2018
Landed in be322bd |
Simplify phrasing for clarity and succinctness. PR-URL: #22024 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Simplify phrasing for clarity and succinctness. PR-URL: #22024 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
ghost commented Jul 31, 2018
Thanks all! |
For a non-English native speaker it's hard to understand what it means.
So make it a common and more clearly speaking.
make -j4 test(UNIX), orvcbuild test(Windows) passes