Closed Bug 1378151 Opened 2 years ago Closed 2 years ago

Remove ifdef from toolbox-process-window.js

Categories

(DevTools :: General, enhancement)

enhancement
Not set

Tracking

(firefox56 fixed)

RESOLVED FIXED
Firefox 56
Tracking Status
firefox56 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

When building devtools add-on, there is no more build step, so we should get rid of all #ifdef. There is still one in the browser toolbox.
Comment on attachment 8883323 [details]
Bug 1378151 - Remove toolbox-process-window.js preprocessing and enable eslint.

https://reviewboard.mozilla.org/r/154222/#review159326

Thanks! 

Not that you *should* do it, but now that the file is not preprocessed we can fix eslint violations and remove it from .eslintignore.
There is a whole section called "# Ignore devtools pre-processed files" which can most likely be completely removed after that.
Attachment #8883323 - Flags: review?(jdescottes) → review+
Comment on attachment 8883323 [details]
Bug 1378151 - Remove toolbox-process-window.js preprocessing and enable eslint.

https://reviewboard.mozilla.org/r/154222/#review159326

(actually we still need to ignore the preferences files, I forgot we opted for parsing the files rather than cleaning the preprocessing instructions)
Comment on attachment 8883323 [details]
Bug 1378151 - Remove toolbox-process-window.js preprocessing and enable eslint.

Here is the eslint fixes merged into the previous patch.
Attachment #8883323 - Flags: review+ → review?(jdescottes)
Comment on attachment 8883323 [details]
Bug 1378151 - Remove toolbox-process-window.js preprocessing and enable eslint.

https://reviewboard.mozilla.org/r/154222/#review159330

::: .eslintignore
(Diff revision 2)
>  devtools/shared/webconsole/test/test_*.html
>  
>  # Ignore devtools pre-processed files
> -devtools/client/framework/toolbox-process-window.js
> -devtools/client/performance/system.js
> -devtools/client/webide/webide-prefs.js

as discussed I think you might have to keep webide-prefs here, but let's see what try says.
Attachment #8883323 - Flags: review?(jdescottes) → review+
It looks like try is green. May be webide-prefs is also subject to different results based on eslint version being used...
Pushed by apoirot@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/82b16c354d52
Remove toolbox-process-window.js preprocessing and enable eslint. r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/82b16c354d52
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.