-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: adopt Microsoft Style Guide officially #34821
Conversation
@nodejs/documentation |
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.
You know my preferences on these style guides, but I have no objection.
doc/guides/doc-style-guide.md
Outdated
|
||
[Javascript type]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Grammar_and_types#Data_structures_and_types | ||
[Microsoft Writing Style Guide]: https://docs.microsoft.com/en-us/style-guide/welcome/ | ||
[`remark-preset-lint-node`]: https://github.com/nodejs/remark-preset-lint-node |
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 mentioned this to @addaleax and I don't know of a more appropriate situation to bring it up other than right here, but I wish we could use scoped packages for these: @nodejs/remark-preset-lint-node
.
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.
@nodejs-core/*
maybe, so we differentiate between external packages and packages targeting collaborators?
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.
[…] differentiate between external packages and packages targeting collaborators
Yes, exactly.
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.
@mmarchini, sorry, not targeting, but rather collaborating on. Why can't we use @nodejs
?
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.
IIRC it was originally reserved on npm so we could use it for new modules/standard library to go under this scope (nodejs/TSC#389 for some of the discussions around this). IMO if we end up using it for that purpose, mixing with tooling that is used exclusively on Node.js core will be confusing, so scoping to something specific for the internal workflow of the project makes more sense.
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.
We have plans for another package-scoped bare specifier: @nodejs/resolver-conformance-tests
.
Refs: https://github.com/nodejs/modules/issues/472#issuecomment-584994986
doc/guides/doc-style-guide.md
Outdated
@@ -104,10 +104,14 @@ this guide. | |||
|
|||
See also API documentation structure overview in [doctools README][]. | |||
|
|||
For topics not covered here, refer to the [Microsoft Writing Style Guide][]. For | |||
issues not covered there, refer to _The Chicago Manual of Style_. |
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.
Add a link to The Chicago Manual of Style?
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 I"m going to remove the Chicago Manual of Style. It can always be added in a later PR. I'm kind of torn about it. On the one hand, it is pretty much the standard for this sort of stuff. On the other hand, it is not freely available online. Links would be to partial versions, or cheatsheets, or a site where you could buy it.
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.
(And we can always add it later.)
PR-URL: #34821 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ricky Zhou <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Derek Lewis <[email protected]>
Landed in bc8a4df |
PR-URL: #34821 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ricky Zhou <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Derek Lewis <[email protected]>
PR-URL: #34821 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ricky Zhou <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Derek Lewis <[email protected]>
PR-URL: #34821 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ricky Zhou <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Derek Lewis <[email protected]>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes