1328693 - Extract Debugger Default prefs

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Jason Laster [:jlast]]

It would be nice if the debugger could manage its own prefs in github.com/devtools-html/debugger.html.

This patch separates out the debugger prefs so that file can be updated when the debugger publishes a new bundle.

Comment 1

[Jason Laster [:jlast]]

Attachment: dbg-prefs.patch

Comment 2

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=43ebb6262d6497f0834e2c0ed1b8fa5ecefa4a25

Comment 3

[Tom Tromey :tromey]

Comment on attachment 8823814 [details] [diff] [review]
dbg-prefs.patch

Review of attachment 8823814 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch.  This looks good to me.
I think, however, that the Services shim will need an update to find the new file:
https://dxr.mozilla.org/mozilla-central/rev/57ac9f63fc6953f4efeb0cc84a60192d3721251f/devtools/client/shared/shim/Services.js#438
Also perhaps the inspector webpack config will need a change.
I'm going to clear the review on this basis.

Comment 4

[Jason Laster [:jlast]]

Attachment: dbg-prefs2.patch

Comment 5

[Jason Laster [:jlast]]

Just updated the prefs pr so that Services loads the debugger prefs... but we should find a good way to test this. Perhaps Julian will know how to use the prefs-loader.

Comment 6

[Tom Tromey :tromey]

Comment on attachment 8823829 [details] [diff] [review]
dbg-prefs2.patch

Review of attachment 8823829 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you, this looks great.

Comment 7

[Tom Tromey :tromey]

(In reply to Jason Laster [:jlast] from comment #5)
> Just updated the prefs pr so that Services loads the debugger prefs... but
> we should find a good way to test this. Perhaps Julian will know how to use
> the prefs-loader.

At some point we'll have integration tests for the inspector bundle; without those
we can confidently assume that the de-chroming will regress.  I think though that
we've agreed to defer this until we think everything is more ready.

Comment 8

[Julian Descottes [:jdescottes]]

I agree with Tom, this should be covered by integration tests and I can't see any easy way to test our Services shim against actual prefs.

I hope we can have integration tests for the inspector as soon as possible . The only thing that concerns me with this approach is that in this context the only prefs we would be testing would be inspector prefs.

Comment 9

[Jason Laster [:jlast]]

Attachment: prefs4.patch

Comment 10

[Jason Laster [:jlast]]

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

Comment 11

[Jason Laster [:jlast]]

Attachment: prefs5.patch

Comment 12

[Jason Laster [:jlast]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fa0b831fdc59a0dfa2c07d3f92ed16c870618c9

Comment 13

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

Comment on attachment 8830300 [details] [diff] [review]
prefs5.patch

Review of attachment 8830300 [details] [diff] [review]:
-----------------------------------------------------------------

Looks logical to me, thanks!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a50557f92553
Extract debugger preferences. r=jryans

Comment 15

[Carsten Book [:Tomcat]]

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

Comment 16

[Jorg K (CEST = GMT+2)]

Ported to TB/IB/SM:
https://hg.mozilla.org/comm-central/rev/1994d82f97322ab5fc42601ad49a859a6844e96b