Closed Bug 1336070 Opened 9 years ago Closed 9 years ago

Enable eslint no-undef for browser/, apart from components/ and base/content/

Categories

(Firefox :: General, defect)

Product:
Firefox
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 54
Tracking Flags:
Tracking Status
firefox54 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(2 files)

Bug 1336070 - Fix no-undef issues in the pocket code.
59 bytes, text/x-review-board-request
mixedpuppy
: review+
Details
Bug 1336070 - Enable eslint no-undef for browser/, apart from components/ and base/content/.
59 bytes, text/x-review-board-request
mossop
: review+
Details
We're now in a position that we can enable no-undef for some of browser/ - everything bar components/ and base/content/. Those directories still need a bit of work for common failures.
Comment on attachment 8832855 [details] Bug 1336070 - Enable eslint no-undef for browser/, apart from components/ and base/content/. https://reviewboard.mozilla.org/r/109098/#review110282 ::: tools/lint/eslint/eslint-plugin-mozilla/lib/rules/import-browserjs-globals.js:63 (Diff revision 1) > - !helpers.getIsHeadFile(this)) { > + let root = helpers.getRootDir(filepath); > + let relativepath = path.relative(root, filepath); > + > + if ((helpers.getTestType(this) != "browser" && > + !helpers.getIsHeadFile(this)) && > + !relativepath.includes("content")) { This starts to open the rule up to content files (in this case, for browser/extensions/pocket/content/main.js). I'm not convinced yet that this is fully the right approach, but I'll know more with the next step of getting some of the content directories passing.
Comment on attachment 8832854 [details] Bug 1336070 - Fix no-undef issues in the pocket code. https://reviewboard.mozilla.org/r/109096/#review110344 ::: browser/extensions/pocket/content/panels/js/saved.js:177 (Diff revision 1) > }, > textToData(text) { > if ($.trim(text).length > 25 || !$.trim(text).length) { > if (text.length > 25) { > myself.showTagsError(myself.dictJSON.maxtaglength); > - changestamp = Date.now(); > + window.changestamp = Date.now(); Does this need to be on window? Can it be on myself?
Attachment #8832854 - Flags: review?(mixedpuppy) → review+
cc/mkaply on pocket stuff
Comment on attachment 8832855 [details] Bug 1336070 - Enable eslint no-undef for browser/, apart from components/ and base/content/. https://reviewboard.mozilla.org/r/109098/#review110350
Attachment #8832855 - Flags: review?(dtownsend) → review+
Comment on attachment 8832854 [details] Bug 1336070 - Fix no-undef issues in the pocket code. https://reviewboard.mozilla.org/r/109096/#review110344 > Does this need to be on window? Can it be on myself? After re-evaluating, it doesn't look like it does. Changed to `this`.
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c34aa5230729 Fix no-undef issues in the pocket code. r=mixedpuppy https://hg.mozilla.org/integration/autoland/rev/fc7f502a43a1 Enable eslint no-undef for browser/, apart from components/ and base/content/. r=mossop
Backed out for failing browser_button_state.js and browser_report_site_issue.js: https://hg.mozilla.org/integration/autoland/rev/ed1c85a67cdac1af3edb0578c9d311023ac0b7a1 https://hg.mozilla.org/integration/autoland/rev/fd0fc3404d168cc62d9e83d24b8999fc92278df3 Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=fc7f502a43a10c0f19f700a33c5d31c6c455bebb Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=74261536&repo=autoland [task 2017-02-03T12:14:50.469178Z] 12:14:50 INFO - TEST-START | browser/extensions/webcompat-reporter/test/browser/browser_button_state.js [task 2017-02-03T12:14:51.546781Z] 12:14:51 INFO - TEST-INFO | started process screentopng [task 2017-02-03T12:14:53.099868Z] 12:14:53 INFO - TEST-INFO | screentopng: exit 0 [task 2017-02-03T12:14:53.100342Z] 12:14:53 INFO - Buffered messages logged at 12:14:50 [task 2017-02-03T12:14:53.106912Z] 12:14:53 INFO - Entering test bound test_button_state_disabled [task 2017-02-03T12:14:53.107199Z] 12:14:53 INFO - Buffered messages finished [task 2017-02-03T12:14:53.108741Z] 12:14:53 INFO - TEST-UNEXPECTED-FAIL | browser/extensions/webcompat-reporter/test/browser/browser_button_state.js | Check that button is enabled for reportable schemes on tab load - Got true, expected false [task 2017-02-03T12:14:53.108983Z] 12:14:53 INFO - Stack trace: [task 2017-02-03T12:14:53.109282Z] 12:14:53 INFO - chrome://mochikit/content/browser-test.js:test_is:911 [task 2017-02-03T12:14:53.110736Z] 12:14:53 INFO - chrome://mochitests/content/browser/browser/extensions/webcompat-reporter/test/browser/browser_button_state.js:test_button_state_disabled:10 [task 2017-02-03T12:14:53.111123Z] 12:14:53 INFO - Tester_execTest@chrome://mochikit/content/browser-test.js:735:9 [task 2017-02-03T12:14:53.113380Z] 12:14:53 INFO - Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:655:7 [task 2017-02-03T12:14:53.114801Z] 12:14:53 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:744:59 [task 2017-02-03T12:14:53.11781
Flags: needinfo?(standard8)
I've just fixed up the test failures, which were due to a scoping issue. Only minor corrections, so pushing again.
Flags: needinfo?(standard8)
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/40e4945412fe Fix no-undef issues in the pocket code. r=mixedpuppy https://hg.mozilla.org/integration/autoland/rev/bd99ca8c230b Enable eslint no-undef for browser/, apart from components/ and base/content/. r=mossop
https://hg.mozilla.org/mozilla-central/rev/40e4945412fe https://hg.mozilla.org/mozilla-central/rev/bd99ca8c230b
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: