Skip to content
This repository has been archived by the owner on Jan 6, 2020. It is now read-only.

Add "Standard" javascript linting #63

Closed
fofr opened this issue Dec 16, 2016 · 9 comments
Closed

Add "Standard" javascript linting #63

fofr opened this issue Dec 16, 2016 · 9 comments

Comments

@fofr
Copy link
Contributor

fofr commented Dec 16, 2016

"Standard" was added to govuk_frontend_toolkit here:
alphagov/govuk_frontend_toolkit#334

By adding Standard to govuk-lint we can lint Javascript across our apps using the same config.

@NickColley @tvararu @dsingleton

@dsingleton
Copy link
Contributor

Sounds reasonable to me.

@NickColley
Copy link
Contributor

Is it possible to do this only for lines changed or similar? I imagine this'll make a lot of projects frustrating for people to update to start with.

@fofr
Copy link
Contributor Author

fofr commented Dec 19, 2016

How can we package up the npm module in this gem?

@fofr
Copy link
Contributor Author

fofr commented Dec 19, 2016

One approach would be to use eslint-rails with the standard JS conf:
https://github.com/appfolio/eslint-rails
https://github.com/feross/eslint-config-standard/blob/master/eslintrc.json

Some challenges I foresee with using eslint-rails:

  • Can't easily customise the files being linted or those being excluded
  • Can't alter location of config file
  • The gem isn't built to be run from within a gem/custom CLI – it does a lot of Rails magic for determining which JS files to lint based on what's in application.js

@fofr
Copy link
Contributor Author

fofr commented May 18, 2017

A WIP to add Javascript linting based on the eslint-rails approach for getting eslint to run in a Ruby environment: https://github.com/alphagov/govuk-lint/compare/javascript-linting

I've stopped this POC because the standard linting config is not simply a list of JSON rules but also a set of plugins that must be included and run. It also feels brittle.

I'd prefer something that uses the npm standard package. I've raised an issue on govuk-puppet regarding the Node version available on Jenkins and VMs: alphagov/govuk-puppet#5942

@tvararu
Copy link

tvararu commented May 18, 2017

@fofr thanks for looking into this Paul!

Now that Rails 5.1 is released with support for webpack and yarn built in, would that be an easier route for this?

@fofr
Copy link
Contributor Author

fofr commented May 18, 2017

@tvararu Yes, I think that’s promising too. It looks like there are some webpack standard libs (eg https://github.com/timoxley/standard-loader).

I'm not sure how webpack/yarn dependencies can be packaged into a gem like govuk-lint.

We'll still need to get Node upgraded first. Requires Node.js 6.4.0+

@fofr fofr mentioned this issue Jun 21, 2017
@dankmitchell
Copy link
Contributor

FYI - We now use XO for linting.

@theseanything
Copy link
Contributor

See #51

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

No branches or pull requests

6 participants