Closed
Bug 1455112
Opened 6 years ago
Closed 6 years ago
Coalesce multiline logs into one line
Categories
(GeckoView :: General, enhancement, P1)
Tracking
(firefox61 fixed)
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: jchen, Assigned: jchen)
References
Details
Attachments
(3 files)
To make it easier to wrap a long log line into multiple lines, we should coalesce multiline literals back into one line. E.g., turn > debug `foo=${foo} > bar=${bar}` into > debug `foo=${foo} bar=${bar}` And use > debug `foo=${foo}` > debug `bar=${bar}` For actually logging multiple lines.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8969074 [details] Bug 1455112 - 1. Coalesce multiple log lines into one; https://reviewboard.mozilla.org/r/237760/#review243738 Nice, thanks! ::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:329 (Diff revision 1) > * do_something(debug.foo = bar); // Output "foo = 42" to the log. > * > * @param tag Name of the Log.jsm logger to forward logs to. > * @param scope Scope to add the logging functions to. > */ > initLogging: function(tag, scope) { aTag, aScope. I somewhat regret our decision to go with prefixes in JS. ::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:358 (Diff revision 1) > this._rootLogger.addAppender(new Log.AndroidAppender()); > } > return this._rootLogger; > }, > > _log: function(logger, level, strings, exprs) { a-prefixes. ::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:369 (Diff revision 1) > } > > + if (logger.level > Log.Level.Numbers[level]) { > + // Log disabled. > + return; > + } That's a different bug, isn't it?
Attachment #8969074 -
Flags: review?(esawin) → review+
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8969075 [details] Bug 1455112 - 2. Add eslint rules to enforce debug/warn usage; https://reviewboard.mozilla.org/r/237762/#review243746 LGTM but I don't know anything about eslint config.
Attachment #8969075 -
Flags: review?(esawin) → review+
Updated•6 years ago
|
Priority: -- → P1
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•6 years ago
|
||
mozreview-review |
Comment on attachment 8969370 [details] Bug 1455112 - 3. Update existing multiline debug lines; https://reviewboard.mozilla.org/r/238120/#review243844
Attachment #8969370 -
Flags: review+
Pushed by nchen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2cd06fff80fe 1. Coalesce multiple log lines into one; r=esawin https://hg.mozilla.org/integration/autoland/rev/345d021363fb 2. Add eslint rules to enforce debug/warn usage; r=esawin https://hg.mozilla.org/integration/autoland/rev/9c43c32dc98e 3. Update existing multiline debug lines; r=jchen
Comment 10•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2cd06fff80fe https://hg.mozilla.org/mozilla-central/rev/345d021363fb https://hg.mozilla.org/mozilla-central/rev/9c43c32dc98e
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Updated•5 years ago
|
Product: Firefox for Android → GeckoView
Updated•5 years ago
|
Target Milestone: Firefox 61 → mozilla61
You need to log in
before you can comment on or make changes to this bug.
Description
•