1097587 - UITour: Add some logging to ease debugging

Status: RESOLVED FIXED

(Firefox :: General, enhancement)

Description

[Matthew N. [:MattN]]

UITour is complex enough that it could use some debug logging to trace what is happening.

I have a patch using console.jsm so that we get rich output for the various Objects which are logged (instead of "[Object object]"). I converted the existing reportError usage so they also get the logging prefix.

Comment 1

[Matthew N. [:MattN]]

Attachment: MozReview Request: bz://1097587/MattN

Comment 2

[Matthew N. [:MattN]]

/r/493 - Bug 1097587 - UITour: Add some logging to ease debugging. r=Unfocused

Pull down this commit:

hg pull review -r e4966c23032b80955a299ac98a3ab7b708a8c8b6

Comment 3

[Matthew N. [:MattN]]

/r/493 - Bug 1097587 - UITour: Add some logging to ease debugging. r=Unfocused

Pull down this commit:

hg pull review -r e4966c23032b80955a299ac98a3ab7b708a8c8b6

Comment 4

[Matthew N. [:MattN]]

/r/493 - Bug 1097587 - UITour: Add some logging to ease debugging. r=Unfocused

Pull down this commit:

hg pull review -r 82a923d03533d5f68d824a88c7ff0c3538992bc4

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Old WiP

I had this old patch sitting around, that adds logging to a few more places that I found useful. You may wish to add these to your patch.

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://reviewboard.mozilla.org/r/493/#review227

Happy with this as-is, but extra happy if you add to it :)

::: browser/modules/UITour.jsm
(Diff revision 2)
> -        }).then(null, Cu.reportError);
> +        }).then(null, log.error);

Bonus points if you convert cases of this to use .catch()

Comment 7

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://reviewboard.mozilla.org/r/491/#review229

Ship It!

Comment 8

[Matthew N. [:MattN]]

Thanks. I added more logging from your patch and switched to .catch

https://hg.mozilla.org/integration/fx-team/rev/8b35d3ba140d

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 8521331 [details]
MozReview Request: bz://1097587/MattN

Approval Request Comment
[Feature/regressing bug #]: Additional logging will help with bug 1084075 for 35.
[User impact if declined]: Developers will be slower at developing the tour and therefore the tour may not be as polished.
[Describe test coverage new/current, TBPL]: Logging-only chages
[Risks and why]: Low risk, logging-only changes that will help with debugging problems => better bug reports and faster bug fixes.
[String/UUID change made/needed]: None

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8b35d3ba140d

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for Aurora uplift.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8521331 [details]
MozReview Request: bz://1097587/MattN

Matt says he isn't going to have time to rebase this any time soon. Given Friday's uplift, clearing the approval to get it off the radar for now.

Comment 13

[Justin Dolske [:Dolske]]

Comment on attachment 8521331 [details]
MozReview Request: bz://1097587/MattN

[Triage Comment]

Needed for Fx35 Hello tour, no unusual risk.

(Already on aurora via the trains, so only beta approval is needed.)

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Still needs rebasing per comment 11...

Comment 15

[Matthew N. [:MattN]]

https://hg.mozilla.org/releases/mozilla-beta/rev/012631bb6655

Comment 17

[Matthew N. [:MattN]]

Attachment: MozReview Request: Bug 1097587 - UITour: Add some logging to ease debugging. r=Unfocused