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

Determine style guide for Javascript #317

Closed
petervanderdoes opened this issue Aug 19, 2016 · 9 comments
Closed

Determine style guide for Javascript #317

petervanderdoes opened this issue Aug 19, 2016 · 9 comments
Assignees
Milestone

Comments

@petervanderdoes
Copy link
Member

Currently the project follows PEP8 style guide for the Python files but no style guide is set for Javascript.

There are several options for the javascript style, this issue should help determine which one will be adopted.

As far as Ecmascript version, I suggest to go with 5 for now. Ecmascript 6 is out but still not fully supported by "older" browsers, IE11, IOS9.

For styling guides we can choose from:

  • AirBnB
  • Google
  • Idiomatic
  • Or something completely different

Point is to choose one and stick with it :)

@rolandgeider
Copy link
Member

I personally don't really really have a preference, for python I chose pep8 because it's kind of the more or less official style guide, so for JS I would like to use something that is similarly known and not do our own thing. Another important thing would be the availability of some kind of automatic testing tool so that we can integrate this with travis.

@petervanderdoes
Copy link
Member Author

Ok, so as a test I decided on AirBnB for the style.

I created a branch on my fork to show what is done so far:

  • Added package.json to control the needed node_modules needed.
  • Use eslint for linting the javascript.
    • Don't error on anonymous functions
    • Work around for functions which are called from a template. eslint thinks these functions are unused and will throw an error. By starting these functions with wger we can tell eslint to not error.
  • Use gulp to automate the linting. (Only nutrition.js is being linted right now)
  • Update the travis.yml
    • Use Node 4
    • Use caching.
    • Use gulp to lint.
  • Only thenutrition.js has been changed to adhere to the new styling guide.

You can see the branch here: https://github.com/petervanderdoes/wger/tree/feature/javascript
And on travis here: https://travis-ci.org/petervanderdoes/wger

Let me know what you think.

@rolandgeider
Copy link
Member

This does indeed look great, I also like the AirBnB style, I think we can keep it :)

@petervanderdoes
Copy link
Member Author

Cool, I'll keep working on it.

@petervanderdoes
Copy link
Member Author

Update

  • All javascript files have been updated to the new coding style
  • Gulp lints all javascript files.
  • Travis lints Python first, javascript second, and Python tests last.
  • Added a coding style document in the docs directory.

Tests returns some problems which I'll be looking into in the next few days.

@petervanderdoes
Copy link
Member Author

All Python tests pass.

@rolandgeider
Copy link
Member

Thanks. I've opened PR #321, I hope I'll find time to take a closer look today, as I am leaving tomorrow for a week for my vacation (I'll have internet and will be able to answer on issues, but not much more). In any case, I've added you to the developers team, that'll make things easier in the future ;)

@petervanderdoes
Copy link
Member Author

Thanks for adding me. I'll not be doing any merging in the near future, at least not without talking to you about it. Enjoy your vacation.

@petervanderdoes petervanderdoes added this to the 1.8 milestone Aug 31, 2016
@petervanderdoes petervanderdoes self-assigned this Aug 31, 2016
@rolandgeider
Copy link
Member

Fixed with #321

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

2 participants