-
Notifications
You must be signed in to change notification settings - Fork 31
Conversation
This is excellent! It addresses basically every meta-point I can remember coming up at the NodeConf docs sessions. Overall 👍, except for a few things:
|
|
||
To summarize: | ||
|
||
* Structure is of paramount important. |
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.
Perhaps "importance"?
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.
Good eye! Fixed in latest.
👍 I like the logical split into guides, topics and reference sections. Other than the "importance" typo and needing to follow its own column-length requirements, it looks good to me. |
OK, ready for another round of review. |
/cc'ing @nodejs/diversity as well, since they might be interested! |
|
||
[as fun]: https://twitter.com/izs/status/187639633641865216 | ||
[Oxford comma]: https://en.wikipedia.org/wiki/Serial_comma | ||
[CONTRIBUTING.md]: CONTRIBUTING.md |
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 am working on the CONTRIBUTING.md guide in a separate PR so that we can incrementally approve changes.
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.
That pull request is here.
LGTM |
LGTM-too |
* US spelling is preferred. "Capitalize" vs. "Capitalise", "color" vs. | ||
"colour", etc. | ||
* Though controversial, the [Oxford comma][] is preferred for clarity's sake. | ||
* Generally avoid personal pronouns in reference documentation ("I", "you", |
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 mention personal pronouns, did we want to make an explicit point regarding using non-gender specific pronounds?
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.
Excellent point! Will fix.
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.
fixed!
Note: Did we want to link back to the overall node.js values once they are made public? I think we should make it explicit/clear that this is a strict subset of the values of the overall org and that those must be respected as well. |
Would it be okay to note this in the CONTRIBUTING guide? The values laid out in this document are scoped to the material and our relation to it: why we're writing it, where we put it, and how we write it. It seems like values (in the sense of project-wide values documenting the community and our relation to it) might fit more comfortably in CONTRIBUTING.md – which also has the nice side effect of being linked on all "create a PR" forms on GitHub. As a separate concern – is |
Could we write some guidance for our markdown syntax limitation? Take an example: We use the following syntax to define
over using the 3 characters "`":
The 2nd one is how we define a parameters of a function:
If we let the docs newcomers know these more syntax rule, that would save time of reviewers. BTW, just curious about why Node.js doesn't use any document generator tool like jsDoc or YUIDoc, that lets maintain the docs and codebase be easy. |
|
||
* Our audiences are **diverse** and have **different goals** coming into the | ||
docs. | ||
* We write docs so that we our audiences can have a **positive experience** with |
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.
typo: "we our audiences"
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.
Fixed – good eye!
@yorkie Noted re: codeblock usage. I haven't documented how we define function name + params yet. I might punt on it until a second revision.
Inline code documentation that is extracted via generators has to address two different audiences by its nature – both the programmer working on the code in-situ, as well as readers who are looking at the docs. It's hard to address both audiences in their current context well, so less is said. It ends up making serviceable reference documentation, but it doesn't address audiences looking for guides or topic material – and because it's inline in the code, it doesn't tend to link very well to those kinds of docs, either. |
repeat that organization internally, so long as it does not interfere with the | ||
communication of the core idea of the document. For example, it's good to add | ||
links to reference materials at the end of topic docs, but it would be unwise | ||
to enumerate a modules entire list of methods inside that same doc. Another |
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.
typo: "modules" -> "module's"
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.
Fixed!
@chrisdickinson yes, I do agree on what you have described on audiences viewpoint, the problem in my previous PR experiences with Node.js is someone forget to update docs when he/she updated the codebase. proposal: is there a possibility to introduce a scan tool to check if the "API" document is match to the current codebase 100%, and add the tool into CI system, then we can check if the PR does lost the related docs. |
# Node Documentation | ||
|
||
Welcome to the Node.js documentation guide – or more colloquially, the docs on | ||
docs. If you're here, you're likely looking for guidance on how to write |
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 like the engaging intro. There are some copyediting nits:
- That looks like an en dash. It should be an em dash. (You can use HTML entities in markdown, so I'd recommend using the mdash entity here.)
- No spaces before or after the dash.
- "Node.js Documentation Guide" rather than "Node.js documentation guide"
- If you think this will be read by many non-native speakers, you may be doing them a favor by avoiding words like colloquially.
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.
That looks like an en dash. It should be an em dash. (You can use HTML entities in markdown, so I'd recommend using the mdash entity here.)
Willfix — it's easy enough to add a literal emdash here (— is Alt+Shift+hyphen on OSX.)
No spaces before or after the dash.
I'm following the NYT usage, where dashes are surrounded in spaces. Maybe I should note that in the mechanical section?
"Node.js Documentation Guide" rather than "Node.js documentation guide"
Willfix.
If you think this will be read by many non-native speakers, you may be doing them a favor by avoiding words like colloquially.
Oof, that is a good point. I had imagined that this doc would be translated by i18n teams, so the translation would only have to happen once. I wonder what the best way to survey non-native speakers is?
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.
Ah! If you're using NY Times guide, I believe en dash rather than em dash is the way to go and (as you note) keep the spaces. So, basically, as you had it and ignore my dash comments.
And, yeah, whatever you use, noting the standard your using will help settle bike shed debates quickly. (And nothing brings out the bike shedding in everyone like copyediting. I'm doing it here!)
Just a note from @nodejs/diversity, this is something we're definitely interested in, and would love to be involved with. At this point in time, however, I wouldn't recommend waiting on us just yet. We're still very new, and I think it will be a while before we have anything finalized and public that can be cross referenced. Once everything is ready, we can circle back and look at collaborating/integrating/etc then. Thanks for pinging us, and I like the direction this document is going in! |
topics/streams/writing-a-stream.md # for authors of streams | ||
topics/streams/index.md # links to both of the above | ||
|
||
In other cases, reference documentation may have need of describing a topic 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.
"may have to describe a topic" is probably easier to understand.
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.
Fixed!
* Code need not be complete — treat code blocks as an illustration or aide to | ||
your point, not as complete running programs. If a complete running program | ||
is necessary, include it as an asset in `assets/code-examples` and link to | ||
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.
I would like to add that any imported modules should either be shown, or the variables should be named as close as possible to those modules.
Another option is if the topic API was shown in an import as part of the high-level description.
Overall Looks good to me, minus comments. I do suggest preferring markdown italics as |
* For illustrations, prefer SVG to other assets. When SVG is not feasible, | ||
please keep a close eye on the filesize of the asset you're introducing. | ||
* For code blocks: | ||
* Use language aware fences. ("```js") |
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.
Not sure this actually works, highlighting broke.
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.
If you view the file it appears to render correctly.
Merging this now. Thanks all! |
add initial version of guide
Adding an initial version of the guide to address #4.
The goal is to relate to documentation team members (and interested contributors) what we write about, how we write it, and why. This should serve as an initial style guide.
Feedback is greatly appreciated!