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

JS style guide enforcement #813

Closed
miketaylr opened this issue Oct 29, 2015 · 9 comments
Closed

JS style guide enforcement #813

miketaylr opened this issue Oct 29, 2015 · 9 comments
Assignees

Comments

@miketaylr
Copy link
Member

We don't really have a style guide, and writing one seems like a lot of work.

It would be cool to hook up some tooling to do the checks for us:

http://eslint.org/docs/rules/ (eslint can also replace jshint)

OR

http://jscs.info/ (to be used in conjunction with jshint).

Mainly I'd like us to have consistent use of quoting and whitespace, and not have to think about these boring things during code review.

Thoughts? @magsout @hallvors

@miketaylr
Copy link
Member Author

Whichever one we pick, we can agree on a set of a few common rules and go from there (and then perhaps send a single fixup PR to catch existing violations)

@hallvors
Copy link
Contributor

jshint is already enforcing some rules, no?

@miketaylr
Copy link
Member Author

Yeah, it does "these things will probably be bugs" rules. I'm also interested in having a consistent codebase in terms of style.

@hallvors
Copy link
Contributor

It's nagging me about "un-necessary semicolon"s now and then. Sounds like a style issue ;)
Anyway, I suggest you just run jscs with a few presets and figure out which of them gives fewer errors with the existing code base.. Then we take it from there, perhaps relaxing some of the rules if we don't like the preset we end up with.

@miketaylr
Copy link
Member Author

It's nagging me about "un-necessary semicolon"s now and then. Sounds like a style issue ;)

OK. But it doesn't do the other interesting things that this issue is about.

I'll mess with jscs or eslint in the coming days and send a PR.

@miketaylr miketaylr self-assigned this Oct 29, 2015
@magsout
Copy link
Member

magsout commented Nov 2, 2015

eslint is a good choice, I use it on a lot of project. Plus I made this https://github.com/stylelint/stylelint-config-cssrecipes for css, so maybe wen can use it too.

But yeah, it's a good idea to have a consistant code base..

@miketaylr
Copy link
Member Author

+1

@magsout
Copy link
Member

magsout commented Nov 3, 2015

@miketaylr if you want we can create a new branch and try to add those linter ?

@miketaylr
Copy link
Member Author

@magsout let's go ahead and open a new issue for CSS linting, I don't think they need to be done in the same branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants