1211483 - `./mach eslint` should warn if `./mach eslint --setup` is not yet run, or just run it

Status: RESOLVED WORKSFORME

(Firefox :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

The error I received when I hadn't run `--setup` was a stack trace with:
 "Error: Cannot find module 'eslint-plugin-mozilla'"

Which is not intuitive.

Patrick, quick fix or mentorable?

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

It might be cleaner just to run it – this would also cover the case where we need to upgrade the dependencies.

Comment 2

[Patrick Brosset <:pbro>]

(In reply to Michael Comella (:mcomella) from comment #1)
> It might be cleaner just to run it – this would also cover the case where we
> need to upgrade the dependencies.
That's a good point.

(In reply to Michael Comella (:mcomella) from comment #0)
> The error I received when I hadn't run `--setup` was a stack trace with:
>  "Error: Cannot find module 'eslint-plugin-mozilla'"
> 
> Which is not intuitive.
> 
> Patrick, quick fix or mentorable?
Forwarding this to Mike who worked on bug 1203520

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Interesting... we are better to check if the plugin exists so we can do that.

Comment 4

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Can you try again with the latest version and report any errors?

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #4)
> Can you try again with the latest version and report any errors?

Me? How can I unset the `mach eslint --setup` state to try this?

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Michael Comella (:mcomella) from comment #5)
> (In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #4)
> > Can you try again with the latest version and report any errors?
> 
> Me? How can I unset the `mach eslint --setup` state to try this?

You don't need to as it will uninstall and then reinstall everything it needs to. You just need to run mach eslint --setup again.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

No errors after running:
 mach eslint --setup
 mach eslint mobile/android

Just to double-check we're on the same page, this bug was initially filed to solve the issue where I ran `mach eslint` without running `--setup` first and I received an unintuitive error.

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #4)
> Can you try again with the latest version and report any errors?

Per Mossop's firefox-dev post on 11/28, it sounds like nothing has changed here -- this is still an issue (for anyone who hasn't ever run ./mach eslint --setup)

Comment 9

[Kris Maglione [:kmag]]

Is there a reason that we have to install these modules globally? The usual practice with node is to provide a top-level package.json, and install the required modules in a top-level node_modules directory.

I only managed to make this work by reading the mach_commands.py source, and manually running the npm commands as root. I actually (inadvisably) tried running `mach eslint --setup` as root, but that required me to also run `mach mercurial-setup` as root, which I wasn't willing to do.

Comment 10

[Kris Maglione [:kmag]]

Er, sorry, that comment wasn't intended for this bug.

Comment 11

[:Gijs (he/him)]

Fairly sure this is now implemented - Running ./mach eslint without --setup complains if things aren't present and runs the setup version for me after confirmation.

Comment 12

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Yup, implemented in bug 1270851.