1378151 - Remove ifdef from toolbox-process-window.js

Status: RESOLVED FIXED

(DevTools :: General, enhancement)

Description

[Alexandre Poirot [:ochameau]]

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 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1378151 - Remove toolbox-process-window.js preprocessing and enable eslint.

Review commit: https://reviewboard.mozilla.org/r/154222/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/154222/

Comment 2

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=794803ee9c04f718c18f8783fa96373e7045ec1f

Comment 3

[Julian Descottes [:jdescottes]]

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.

Comment 4

[Julian Descottes [:jdescottes]]

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 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 8883323 [details]
Bug 1378151 - Remove toolbox-process-window.js preprocessing and enable eslint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/154222/diff/1-2/

Comment 6

[Alexandre Poirot [:ochameau]]

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.

Comment 7

[Julian Descottes [: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.

Comment 8

[Alexandre Poirot [:ochameau]]

It looks like try is green. May be webide-prefs is also subject to different results based on eslint version being used...

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/82b16c354d52
Remove toolbox-process-window.js preprocessing and enable eslint. r=jdescottes

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/82b16c354d52