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

v5: Linting / Code styles #2377

Closed
oliverfoster opened this issue Mar 7, 2019 · 5 comments
Closed

v5: Linting / Code styles #2377

oliverfoster opened this issue Mar 7, 2019 · 5 comments

Comments

@oliverfoster
Copy link
Member

Subject of the issue

@oliverfoster
Copy link
Member Author

I'm not sure about using standardjs as a package. I think if we stick with eslint and start with the standardjs config removing the no colons requirement.
All of us use colons, it'll get in the way even though it makes a lot of sense to only use them where they're necessary.

@tomgreenfield
Copy link
Contributor

@nilslp
Copy link
Contributor

nilslp commented Mar 14, 2019

Current reference: https://github.com/adaptlearning/documentation/blob/master/01_cross_workstream/style_guide.md.

Is this something that we will need to keep or will we be running with the standard eslint config as @oliverfoster has mentioned?

@moloko
Copy link
Contributor

moloko commented Mar 15, 2019

@nilslp i’d be tempted to keep that so that people can view the style guide in ‘human readable’ format rather than having to work through a config file trying to figure out what the rules are

@oliverfoster agree that eslint looks like the better option

@oliverfoster
Copy link
Member Author

oliverfoster commented Mar 15, 2019

@nilslp found https://prettier.io/ which is a code formatter rather than a linter. It's got good integration with editors and git hooks.
Instead of requiring people to code in the right style the style will be applied automatically, on save with an extension or on commit with git hooks.
It doesn't do linting so won't help with errors but it does apply the style instead of reporting it and it does all the languages we need, whereas ESLint only does JS.
Might be a better idea?

brian-learningpool added a commit that referenced this issue Jun 26, 2019
Includes .eslintrc file defaulted to standardjs with some caveats, e.g. 4 spaces rather than 2, semicolons allowed, etc.  (See 'rules' for details.)

Some common common globals (Backbone, underscore) have been added but there are bound to be more required.
oliverfoster pushed a commit that referenced this issue Feb 20, 2020
* Resolves #2377, adds ESLint as a dev dependency

Includes .eslintrc file defaulted to standardjs with some caveats, e.g. 4 spaces rather than 2, semicolons allowed, etc.  (See 'rules' for details.)

Some common common globals (Backbone, underscore) have been added but there are bound to be more required.

* Added .eslintignore file

* Removed JSCS and JSHint

* Added 'prettier'

This should go some way to relieving coding style issues.

* Switched to 2 space indentation, consistent with v5 branch

Also changed line width from 80 characters.

* Removed 'prettier'

After some evaluation, I think this is too opinionated for our codebase.

* Added 'npm run lint' command

* Changes from manual lint fix

* Correct .eslintignore

All .js files in /src/core/libraries are now being ignored.

* More manual fixes

Also allowing ES6 code in /grunt folder.

* Added enforcement of semi-colons to ESLint rules

* Reverted formatting on ternary operators

* Added operator-linebreak to .eslintrc

This preserves existing ternary operators.

Also, minor fixes.

* Cleared final two linting errors

Also removed .prettierrc.json, as this was not required.
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

4 participants