Skip to content

tools: enable space-after-keywords eslint rule#2430

Closed
brendanashworth wants to merge 2 commits into
nodejs:masterfrom
brendanashworth:new/linter-rules
Closed

tools: enable space-after-keywords eslint rule#2430
brendanashworth wants to merge 2 commits into
nodejs:masterfrom
brendanashworth:new/linter-rules

Conversation

@brendanashworth
Copy link
Copy Markdown
Contributor

This adds a new eslint rule: space-after-keywords.

Spaces after keywords makes this error (6 now fixed errors):

if(x) { ... }

in favor of:

if (x) { ... }
@brendanashworth brendanashworth added the tools Issues and PRs related to the tools directory. label Aug 18, 2015
@cjihrig
Copy link
Copy Markdown
Contributor

cjihrig commented Aug 18, 2015

I like it. LGTM

@targos
Copy link
Copy Markdown
Member

targos commented Aug 19, 2015

LGTM

@bnoordhuis
Copy link
Copy Markdown
Member

Not a big fan of new-cap. new clazz() makes it clear you're instantiating from a variable, new Clazz() not so much.

@brendanashworth
Copy link
Copy Markdown
Contributor Author

new clazz() makes it clear you're instantiating from a variable, new Clazz() not so much.

Would you prefer the tests be refractored so no arguments are called with new?

@bnoordhuis
Copy link
Copy Markdown
Member

No, I just don't think newIsCap is a very good rule. I'd leave it out.

eg changes:

  if(x) { ... }

to:

  if (x) { ... }
Requires that you do:

  if (x) { ... }

Rather than:

  if(x) { ... }
@brendanashworth
Copy link
Copy Markdown
Contributor Author

PTAL @bnoordhuis, i've removed the rule.

@brendanashworth brendanashworth changed the title Enable 2 new eslint rules tools: enable space-after-keywords eslint rule Aug 22, 2015
@bnoordhuis
Copy link
Copy Markdown
Member

LGTM

@brendanashworth
Copy link
Copy Markdown
Contributor Author

Thank you, landed in 8af5962 and 96a2b2d.

@cjihrig
Copy link
Copy Markdown
Contributor

cjihrig commented Aug 23, 2015

@brendanashworth did you include the commit metadata on those?

@brendanashworth
Copy link
Copy Markdown
Contributor Author

Oh no, looks like I missed that.. a little bit late but thank you for
catching it. I'll comment on the commits.

On Saturday, August 22, 2015, Colin Ihrig notifications@github.com wrote:

@brendanashworth https://github.com/brendanashworth did you include the
commit metadata on those?


Reply to this email directly or view it on GitHub
#2430 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools Issues and PRs related to the tools directory.

4 participants