1349009 - [ESLint] Clean up responsivedesign.jsm

Status: RESOLVED FIXED

(DevTools :: Responsive Design Mode, defect, P3)

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1349009 - Fix ESLint errors in responsivedesign.jsm.

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

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8849329 [details]
Bug 1349009 - Fix ESLint errors in responsivedesign.jsm.

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

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=95356a76f25251ac607ab646f2abb1479e93646e

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

This ended up finding the `flags` module that was never required here...  This might fix some intermittent tests, since we weren't setting testing mode correctly.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9035846b292c47b8e83f9d9cf52d705aabc85d9e

Comment 6

[Tom Tromey :tromey]

Comment on attachment 8849329 [details]
Bug 1349009 - Fix ESLint errors in responsivedesign.jsm.

https://reviewboard.mozilla.org/r/122126/#review124474

Thank you.  This looks good to me.

::: devtools/client/responsivedesign/responsivedesign.jsm:249
(Diff revision 2)
>      try {
>        if (Services.prefs.getBoolPref("devtools.responsiveUI.rotate")) {
>          this.rotate();
> +      }
> +    } catch (e) {
> +      // Ignored

If it can fail, should we log the failure?

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4)
> This ended up finding the `flags` module that was never required here... 
> This might fix some intermittent tests, since we weren't setting testing
> mode correctly.

Oops, looks like `flags` was actually there before, I must have removed it by accident while editing... x_x Anyway, at least the linting told me about it...!

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8849329 [details]
Bug 1349009 - Fix ESLint errors in responsivedesign.jsm.

https://reviewboard.mozilla.org/r/122126/#review124474

> If it can fail, should we log the failure?

It can fail since there's no default value for it, so failure is actually the default path...  Since this code is hopefully removed soon anyway, I'll clarify the comment (rather than adding a default value).

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8849329 [details]
Bug 1349009 - Fix ESLint errors in responsivedesign.jsm.

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ef9b3cb0421e
Fix ESLint errors in responsivedesign.jsm. r=tromey

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ef9b3cb0421e