Pick a coding standard - Githubissues

OSDG-IIITH / www

This repository is the landing page for osdg.iiit.ac.in.

Other

4 stars 16 forks source link

Pick a coding standard #12

Open ffledgling opened 9 years ago

ffledgling commented 9 years ago

[ ] Look at possible options for a coding standard
[ ] Discussion pros & cons in this issue
[ ] Pick one and stick with it, preferably one with a linter/syntax checker

As a follow up, once this is done, we can add a pre-commit hook that runs the syntax checker.

nisargjhaveri commented 9 years ago

I think the following would be good

PSR-2 for PHP
JSHint with default config including enforcing parameters for JavaScript. (Only option I'm not sure with is "strict" as I don't think many of us uses or are comfortable with strict mode)

Everything is open to discussion. Thoughts?

ffledgling commented 9 years ago

I would suggest keeping strict mode on. It's a requirement for ES5 iirc, and based on my understanding and use all it basically does is, is that it forces you to declare your variables with var before you use them so that everything doesn't go to global scope.

nisargjhaveri commented 9 years ago

It also does some more things too. See this. But I'm okay with it if people think it is good to have.

Also, the declaration of local variables will be checked even if the strict mode is off with undef option.

ffledgling commented 9 years ago

Yup, I'm aware of the other effects, but in practice the var declaration issue is all I've encountered is what I was trying to say. I'll defer this one to people who'll be affected by it most.

amoghbl1 commented 9 years ago

@ffledgling instead of using a hook for testing this, I think we should use travis-ci instead of pre-commit hooks for testing standards compliance.

ffledgling commented 9 years ago

@amoghbl1 we can probably add it to travis-ci as well, but a pre-commit hook makes more sense to me because it'll run on the local machine and warn whoever is trying to commit that they messed up syntax. Getting to know after you've pushed and waited for Travis-CI isn't great, especially for something like syntax.

nisargjhaveri commented 9 years ago

I think we should have both. The pre-commit hook for dev, and some online CI tool for reviewer.

Also, look at http://scrutinizer-ci.com/ too. It is pretty nice for code quality checks.

amoghbl1 commented 9 years ago

@ffledgling, it is not standard conventions to add hooks into repositories. Also, they aren't actually added in the repo. So the contributors would need to manually set that up after they clone the repo. But yes, that would be a convenient tool to have for people who work on it, although you need to do more work to set it up. We could probably add that in the readme, and then use travis-ci to actually check if the PR's that are sent pass the tests. @nisargjhaveri I haven't looked into http://scrutinizer-ci.com/ and really want to mess around with travis!! Will see which one would be better for this project and let's use that one.