Skip to content
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 : being more explicit in the synopsis #17977

Closed
wants to merge 9 commits into from
Closed

Conversation

timotew
Copy link
Contributor

@timotew timotew commented Jan 4, 2018

Assuming less knowledge on the part of the reader, making it easier to for starters.

Fixes: #17970, nodejs/help#1049

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

fs

Assuming less knowledge on the part of the reader, making it easier to get start using Node.js.
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jan 4, 2018

Next, create a new source file in the `projects` folder and call it `hello_world.js`.

If you’re using more than one word in your filename, use an underscore(`_`) to separate them for simplicity and avoid using the space character in file names.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe "use an underscore (_) or a hyphen (-)"? Some style guides and practices (including Node.js repository) prefer hyphens, so we can mention both here.

`'Hello World'`:
`'Hello World!'`:

Firstly, Make sure you have downloaded Node.js from [Node.js Official website](http://nodejs.org/#download).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make -> make?

`'Hello World!'`:

Firstly, Make sure you have downloaded Node.js from [Node.js Official website](http://nodejs.org/#download).
Then, Follow this [installation guide](https://nodejs.org/en/download/package-manager/).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow -> follow?

Then, Follow this [installation guide](https://nodejs.org/en/download/package-manager/).


Now, Create an empty project folder called `projects`, navigate into it:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create -> create?


If you’re using more than one word in your filename, use an underscore(`_`) to separate them for simplicity and avoid using the space character in file names.

For example, you’d use `hello_world.js` rather than `hello world.js`.
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Jan 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually do not use personal pronouns in the docs, but this guide is a less formal one, so I am not sure if this is OK. Let's see what others think.

For example, you’d use `hello_world.js` rather than `hello world.js`.
Node.js files always end with the `.js` extension.

open `hello_world.js` in your favorite text editor and paste in the following content.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open -> Open?


To run the server, put the code into a file called `example.js` and execute
it with Node.js:
Save the file, and go back to your terminal window enter the following command:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Save the file, go back to your terminal window and enter?

```
Your should see an output like this in your terminal to indicate Node.js server is running:
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Jan 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your -> you? (If pronouns are considered appropriate).

```javascript
Server running at http://127.0.0.1:3000/
````
Now, Open your favorite browser and visit `http://127.0.0.1:3000`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open ->open?

Copy link
Contributor

@vsemozhetbyt vsemozhetbyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some nits, but let's see what native speakers think.

Thank you!

@gibfahn
Copy link
Member

gibfahn commented Jan 4, 2018

We usually do not use personal pronouns in the docs, but this guide is a less formal one, so I am not sure if this is OK. Let's see what others think.

I think it's fine here, this is directly addressed to the user "How you can run these code samples".

Copy link
Member

@gibfahn gibfahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good

`'Hello World!'`:

Firstly, Make sure you have downloaded Node.js from [Node.js Official website](http://nodejs.org/#download).
Then, Follow this [installation guide](https://nodejs.org/en/download/package-manager/).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change:

Then, Follow this

to

If you want to install Node using a package manager, see [this guide]

If people are just getting started with Node they probably don't need to know about package managers.

Now, Create an empty project folder called `projects`, navigate into it:

Linux and Mac:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include something like this:

We’ll be showing off a number of commands using a terminal, and those lines all start with $. You don’t need to type in the $ character; they are there to indicate the start of each command. You’ll see many tutorials and examples around the web that follow this convention: $ for commands run as a regular user, and # for commands you should be running as an administrator. Lines that don’t start with $ are typically showing the output of the previous command.

(taken from https://doc.rust-lang.org/book/second-edition/ch01-01-installation.html#installation, feel free to reword it).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gibfahn Note my previous comments about personal pronouns. I wonder if this should really be a guide so that we don't have to worry about that sort of thing. Link to it from the docs?

Then, Follow this [installation guide](https://nodejs.org/en/download/package-manager/).


Now, Create an empty project folder called `projects`, navigate into it:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tweak this to make it clear that projects is just an example name, not one that has any significance (i.e. you don't have to call it projects)

Maybe:

If you aren't sure where to put your code, a good place to start is to create a projects folder in your home directory like so:

(
I took inspiration from the rust book again, specifically here:

First, make a directory to put your Rust code in. Rust doesn’t care where your code lives, but for this book, we’d suggest making a projects directory in your home directory and keeping all your projects there. Open a terminal and enter the following commands to make a directory for this particular project:

https://doc.rust-lang.org/book/second-edition/ch01-02-hello-world.html#creating-a-project-directory
)

`'Hello World!'`:

Firstly, make sure you have downloaded Node.js from [Node.js Official website](http://nodejs.org/#download).
Then, follow this [installation guide](https://nodejs.org/en/download/package-manager/).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change:

Then, Follow this

to

If you want to install Node using a package manager, see [this guide]

If people are just getting started with Node they probably don't need to know about package managers.

Now, create an empty project folder called `projects`, navigate into it:

Linux and Mac:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include something like this:

We’ll be showing off a number of commands using a terminal, and those lines all start with $. You don’t need to type in the $ character; they are there to indicate the start of each command. You’ll see many tutorials and examples around the web that follow this convention: $ for commands run as a regular user, and # for commands you should be running as an administrator. Lines that don’t start with $ are typically showing the output of the previous command.

(taken from https://doc.rust-lang.org/book/second-edition/ch01-01-installation.html#installation, feel free to reword it).


Next, create a new source file in the `projects` folder and call it `hello-world.js`.

If you’re using more than one word in your filename, use an hyphen (`-`) or an underscore(`_`) to separate them for simplicity and avoid using the space character in file names.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you wrap lines at 80 chars?

```
you should see an output like this in your terminal to indicate Node.js server is running:
```javascript
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove the javascript here.

Then, follow this [installation guide](https://nodejs.org/en/download/package-manager/).


Now, create an empty project folder called `projects`, navigate into it:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make it more clear that your folder doesn't have to be called projects? Maybe:

Create a directory to put your code in, for example to create a projects folder in your home directory you can run these commands in a terminal:

@gibfahn
Copy link
Member

gibfahn commented Jan 4, 2018

@the-wazz could you let us know whether this is clear for you?


Now, create an empty project folder called `projects`, navigate into it:

Linux and Mac:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be UNIX rather than Linux and Mac ?

Copy link
Member

@Trott Trott Jan 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually go with POSIX but people might not know what that means. UNIX is technically incorrect because that refers to a specific trademarked operating system. UNIX-like is often used instead. Maybe Unix/macOS to conform with BUILDING.md even though I don't like that personally. This gets tricky. Maybe leave it as Linux and Mac after all. :-D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doc/api> grep -i "UNIX" * | wc -l
      40

We seem to be using UNIX abundantly to refer UNIX-like systems.

If we use Linux and MAC, it leaves an impression of either:

  • Only Linux and MAC are supported
  • The example holds good only for Linux and MAC

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gireeshpunathil I'm convinced. Let's go with UNIX (or Unix or UNIX-like) and consolidating terminology/typography across documents can happen some other time. Certainly "Linux and macOS" would explicitly exclude (for example) AIX which certainly is incorrect in this case.


they are there to indicate the start of each command.

You’ll see many tutorials and examples around the web that follow this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Micro-nit (non-blocking, ignore if you disagree): remove around the web

`'Hello World!'`:

We’ll be displaying a number of commands using a terminal, and those lines

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs are usually single-spaced. I'm not sure that double-spacing like this will render the same as other docs. Probably best to single-space it. (Blank lines between paragraphs are 👍 though!)

@Trott
Copy link
Member

Trott commented Jan 4, 2018

I almost feel bad leaving this comment, but since this is a doc and not an informal guide, we try to avoid personal pronouns like you (although I'm sure there are many examples where we don't manage to avoid it). If you can re-word some or all of it, it might fit the tone of other docs better.

For example, instead of:

We’ll be displaying a number of commands using a terminal, and those lines
start with $ or >. You don’t need to type in the $ and > character;
they are there to indicate the start of each command.

...maybe more like this?:

Commands displayed in this document are shown starting with $ or >
to replicate how they would appear in a user's terminal. Do not include the $
or > when running these commands.

@Trott
Copy link
Member

Trott commented Jan 4, 2018

(By the way, the personal pronoun thing and a bunch of other rules/guidelines can be found at https://github.com/nodejs/node/blob/master/doc/STYLE_GUIDE.md.)


and call it `hello-world.js`.

If you’re using more than one word in your filename, use an hyphen (`-`) or
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/an hyphen/a hyphen/

@Trott
Copy link
Member

Trott commented Jan 5, 2018

Wow! You managed to get rid of all the personal pronouns. I'm kind of amazed. :-D


Commands displayed in this document are shown starting with `$` or `>`
to replicate how they would appear in a user's terminal.
Do not include the `$` and `>` character they are there to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Period after character.

@Trott
Copy link
Member

Trott commented Jan 5, 2018

Thanks for doing this!

Lines that don’t start with `$` or `>` character are typically showing
the output of the previous command.

Firstly, make sure to have downloaded Node.js from [Node.js Official website](http://nodejs.org/#download).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the link itself down to the end of the document

```
an output like this should appear in the terminal to indicate Node.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/an/An

Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments. Besides that LG.


To install Node using a package manager, see [this guide][].


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: multiple lines are actually going to be shown as a single line in the output. The same applies to line 67.

Lines that don’t start with `$` or `>` character are typically showing
the output of the previous command.

Firstly, make sure to have downloaded Node.js from [Node.js Official website][].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally do not like to recommend the regular download as it makes it more difficult for the user to switch to a new version. Therefore I would prefer to directly reference the guide.
It could be written as e.g.:
Firstly, make sure to have downloaded and installed Node.js. See [this guide][] for further install information.


Many of the examples in the documentation can be run similarly.

[Command Line Options]: cli.html#cli_command_line_options
[web server]: http.html
[this guide]:https://nodejs.org/en/download/package-manager/
[Node.js Official website]:http://nodejs.org/#download
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a empty space after the colon.

$ mkdir ~/projects
$ cd ~/projects
```
Windows CMD:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is always good to have a blank line before and after code / markdown blocks etc.

the space character in file names.

For example, use `hello-world.js` rather than `hello world.js`.
Node.js files always end with the `.js` extension.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally do not feel like this limitation should be enforced, no matter that it is good style. The point for me would be to educate the user by telling why and that is missing here.
So instead of these two paragraphs I would write e.g.:

In Node.js it is considered good style to use hypens (`-`) or underscores (`_`) to separate
multiple words in filenames.

Besides that not only Node.js files end with .js but any JavaScript file. Is that part really necessary to be documented?

});

server.listen(port, hostname, () => {
console.log(`Server running at http://${hostname}:${port}/`);
});
```
Save the file, go back to the terminal window enter the following command:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: As above: a blank line before and after code blocks is always good. There are more of these cases below.

@BridgeAR BridgeAR removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 19, 2018
Copy link
Member

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the last two open comments fixed.

Next, create a new source file in the `projects` folder
and call it `hello-world.js`.

In Node.js it is considered good style to use hyphens (`-`) or underscores (`_`) to separate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this exceeds 80 characters.

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 6, 2018
@BridgeAR
Copy link
Member

BridgeAR commented Feb 6, 2018

It would be nice to address @Trott s comment while landing.

@BridgeAR
Copy link
Member

BridgeAR commented Feb 6, 2018

BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Feb 7, 2018
Assuming less knowledge on the part of the reader, making it easier
to get start using Node.js.

PR-URL: nodejs#17977
Fixes: nodejs#17970,
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@BridgeAR
Copy link
Member

BridgeAR commented Feb 7, 2018

Landed in 4ac7be9 🎉

@BridgeAR BridgeAR closed this Feb 7, 2018
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Assuming less knowledge on the part of the reader, making it easier
to get start using Node.js.

PR-URL: #17977
Fixes: #17970,
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Assuming less knowledge on the part of the reader, making it easier
to get start using Node.js.

PR-URL: #17977
Fixes: #17970,
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
Assuming less knowledge on the part of the reader, making it easier
to get start using Node.js.

PR-URL: #17977
Fixes: #17970,
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 21, 2018
gibfahn pushed a commit that referenced this pull request Apr 13, 2018
Assuming less knowledge on the part of the reader, making it easier
to get start using Node.js.

PR-URL: #17977
Fixes: #17970,
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
gibfahn pushed a commit that referenced this pull request Apr 13, 2018
Assuming less knowledge on the part of the reader, making it easier
to get start using Node.js.

PR-URL: #17977
Fixes: #17970,
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Apr 13, 2018
@MylesBorins MylesBorins mentioned this pull request May 2, 2018
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
Assuming less knowledge on the part of the reader, making it easier
to get start using Node.js.

PR-URL: nodejs#17977
Fixes: nodejs#17970,
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Be more explicit in doc/api/synopsis.md