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

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

Closed

Conversation

brendanashworth
Copy link
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
Contributor

cjihrig commented Aug 18, 2015

I like it. LGTM

@targos
Copy link
Member

targos commented Aug 19, 2015

LGTM

@bnoordhuis
Copy link
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
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
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
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
Member

LGTM

@brendanashworth
Copy link
Contributor Author

Thank you, landed in 8af5962 and 96a2b2d.

@cjihrig
Copy link
Contributor

cjihrig commented Aug 23, 2015

@brendanashworth did you include the commit metadata on those?

@brendanashworth
Copy link
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants