Closed Bug 1217922 Opened 6 years ago Closed 6 years ago

eslint head.js plugin does not seem to work

Categories

(DevTools :: General, defect)

defect
Not set
normal

Tracking

(firefox45 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

(Reporter: tromey, Assigned: miker)

References

Details

Attachments

(1 file)

I have the eslint plugins installed and I have emacs set up to run
eslint.  This works in most cases.

However, the head.js plugin does not seem to work.
I opened

devtools/client/styleinspector/test/browser_computedview_keybindings_01.js

and I see multiple eslint errors; here's a brief (and mildly cryptic) summary:

   19   9 error    no-... "addTab" is not defined.... (javascript-eslint)
   20  33 error    no-... "openComputedView" is not defined.... (javascript-eslint)
   21   9 error    no-... "selectNode" is not defined.... (javascript-eslint)
   28  32 error    no-... "once" is not defined.... (javascript-eslint)
See Also: → 1217851
Interestingly, it didn't work for me on Windows, but after I fixed the path as described in bug 1217851 it did work just fine. All globals like addTab were being defined.
Assignee: nobody → mratcliffe
@Tom: Was this on a Windows box?
Flags: needinfo?(ttromey)
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #2)
> @Tom: Was this on a Windows box?

Nope, Linux.
Flags: needinfo?(ttromey)
It does not appear to work for me on Mac either.
Can you try again with the latest version and report any errors?
I tried it today.

It still seems to fail.  I opened devtools/client/webconsole/test/browser_webconsole_output_02.js
I get some warnings which are correct, and then also:

  181  25 error    no-... "loadTab" is not defined.... (javascript-eslint)
  182  23 error    no-... "openConsole" is not defined.... (javascript-eslint)
  183  11 error    no-... "checkOutputForInputs" is not defined.... (javascript-eslint)
  185  11 error    no-... "finishTest" is not defined.... (javascript-eslint)

All of these are defined in the corresponding head.js.
This should hopefully be fixed by bug 1225289.
Depends on: 1222232
No longer depends on: 1225289
We are getting the process's working directory but on some OSes that is not necessarily the same as the target file's directory.

I believe that I have a simple fix.
Bug 1217922 - eslint head.js plugin does not seem to work r?=pbrosset

(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #12)
> Comment on attachment 8690134 [details]
> MozReview Request: Bug 1217922 - eslint head.js plugin does not seem to work
> r?=pbrosset
>
> https://reviewboard.mozilla.org/r/25771/#review23289
>
> ::: testing/eslint-plugin-mozilla/lib/rules/import-headjs-globals.js:95
> (Diff revision 1)
> > -      checkFile([testPath, headjs], context);
> > +      checkFile([fullTestPath, fullHeadjsPath]);
>
> You're not passing context anymore here as the 2nd argument. Is this
> intentional?
> The checkFile function's signature wasn't changed in this commit:
> `function checkFile(fileArray, context)`
> so it will still expect a `context` parameter.

Well spotted... this is a simple mistake caused by breaking apart a much larger patch.
Attachment #8690773 - Flags: review?(pbrosset)
Attachment #8690773 - Flags: review?(pbrosset) → review+
Comment on attachment 8690773 [details]
MozReview Request: Bug 1217922 - eslint head.js plugin does not seem to work r?=pbrosset

https://reviewboard.mozilla.org/r/25943/#review23333
https://hg.mozilla.org/mozilla-central/rev/8739d8da1b0d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.