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: make comment indentation consistent #9518

Closed
wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Nov 8, 2016

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

doc

Description of change

Currently, console.md use different indentation for comments in the code
examples. This commit makes the indentation consistent by putting the
comments at the beginning of the line (really no indentation that is). I
was not sure what is preferred but hopefully comments on the PR will
provide guidance..

@nodejs-github-bot nodejs-github-bot added console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations. labels Nov 8, 2016
@silverwind
Copy link
Contributor

Looking good from my side, but did you check other doc files that this is the only case of such comments? I think there ought to be more similar cases.

@danbev
Copy link
Contributor Author

danbev commented Nov 9, 2016

I'd be happy to go the through the rest, but before I do are people happy
with no indentation or would a two space indentation be preferred?

onsdag 9 november 2016 skrev silverwind notifications@github.com:

Looking good from my side, but did you check other doc files that this is
the only case of such comments? I think there ought to be more similar
cases.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#9518 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAaY39r1zJu79rwIR5gYT9HDhXzOsgvcks5q8g1-gaJpZM4Ks2q6
.

@silverwind
Copy link
Contributor

@nodejs/collaborators any preferences for style of indentation in code samples? I personally prefer the non-indented version.

@mscdex
Copy link
Contributor

mscdex commented Nov 9, 2016

IMHO the comments should be above the line it's referring to and unindented.

@silverwind
Copy link
Contributor

IMHO the comments should be above the line it's referring to and unindented.

I generally agree, but think an exception for stdout could be made. Some people use this style for those:

console.log('hello world');
//=> hello world

@targos
Copy link
Member

targos commented Nov 9, 2016

Unindented comments LGTM.
I would also prefer to have them before the corresponding line but have no strong opinion.

@gibfahn
Copy link
Member

gibfahn commented Nov 9, 2016

Comments look good unindented

+1 for above the line, I get the argument for having stdout below, but I think it's less clear.

Copy link
Contributor

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

LGTM, nice work. I prefer the comment-after-code style because it's what one would see in the REPL. Let's see if we get a few more approvals.

@danbev danbev force-pushed the consistent-indent-of-comments branch from 80e2d81 to 6a1f03c Compare November 15, 2016 04:54
@danbev
Copy link
Contributor Author

danbev commented Nov 15, 2016

@danbev
Copy link
Contributor Author

danbev commented Nov 15, 2016

I'd like to merge this later today if there are no objections.

Currently, console.md use different indentation for comments in the code
examples. This commit makes the indentation consistent by putting the
comments at the beginning of the line (really no indentation that is). I
was not sure what is preferred but hopefully comments on the PR will
provide guidance..
@danbev danbev force-pushed the consistent-indent-of-comments branch from 6a1f03c to 867deda Compare November 16, 2016 06:47
danbev added a commit to danbev/node that referenced this pull request Nov 16, 2016
Currently, some of the docs use different indentation for comments
in the code examples. This commit makes the indentation consistent
by putting the comments at the beginning of the line (really no
indentation that is).

PR-URL: nodejs#9518
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@danbev
Copy link
Contributor Author

danbev commented Nov 16, 2016

Landed in 367065b

@danbev danbev closed this Nov 16, 2016
addaleax pushed a commit that referenced this pull request Nov 22, 2016
Currently, some of the docs use different indentation for comments
in the code examples. This commit makes the indentation consistent
by putting the comments at the beginning of the line (really no
indentation that is).

PR-URL: #9518
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 13, 2016
Currently, some of the docs use different indentation for comments
in the code examples. This commit makes the indentation consistent
by putting the comments at the beginning of the line (really no
indentation that is).

PR-URL: #9518
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 21, 2016
Currently, some of the docs use different indentation for comments
in the code examples. This commit makes the indentation consistent
by putting the comments at the beginning of the line (really no
indentation that is).

PR-URL: #9518
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 21, 2016
@danbev danbev deleted the consistent-indent-of-comments branch January 17, 2017 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants