1440346 - Add a legend to the ASCII art tree that SyncedBookmarkMirror outputs in the log

Status: RESOLVED FIXED

(Firefox :: Sync, enhancement, P3)

Description

[Thom Chiovoloni [:tcsc] (ex-moco)]

It's fairly opaque as it is. A short legend would go a long way.

Comment 1

[Thom Chiovoloni [:tcsc] (ex-moco)]

In IRC markh requested that these include the URLs for bookmarks as well (mostly for mnemonic value). These are already only logged on TRACE so the additional PII isn't a problem. It might make sense to log folder names as well.

It's possible we'd want to avoid logging more than N (for some smallish N) characters of urls though.

Comment 2

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment #1 should instead be done as part of bug 1444304, since it's nontrivial.

Comment 3

[Thom Chiovoloni [:tcsc] (ex-moco)]

I'm doing this now since I want better logs.

Comment 4

[Thom Chiovoloni [:tcsc] (ex-moco)]

Attachment: Bug 1440346 - Replace abbreviated strings in the sync bookmark mirror's tree log with full names

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

Comment 5

[Lina Butler (ex-Mozilla)]

Comment on attachment 8957719 [details]
Bug 1440346 - Replace abbreviated strings in the sync bookmark mirror's tree log with full names

https://reviewboard.mozilla.org/r/226674/#review232470

Thanks!

::: toolkit/components/places/SyncedBookmarksMirror.jsm:3062
(Diff revision 1)
>     * are prefixed with "~".
>     *
>     * @return {String}
>     */
>    toASCIITreeString() {
>      return this.root.toASCIITreeString() + "\n" + Array.from(this.deletedGuids,

Should we change this to `Deleted: A, B, C`, instead of `~A (deleted), ~B (deleted), ~C (deleted)`?

Comment 6

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8957719 [details]
Bug 1440346 - Replace abbreviated strings in the sync bookmark mirror's tree log with full names

https://reviewboard.mozilla.org/r/226674/#review232470

> Should we change this to `Deleted: A, B, C`, instead of `~A (deleted), ~B (deleted), ~C (deleted)`?

Good point!

Comment 7

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8957719 [details]
Bug 1440346 - Replace abbreviated strings in the sync bookmark mirror's tree log with full names

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

Comment 8

[Thom Chiovoloni [:tcsc] (ex-moco)]

Comment on attachment 8957719 [details]
Bug 1440346 - Replace abbreviated strings in the sync bookmark mirror's tree log with full names

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30b4097734ce
Replace abbreviated strings in the sync bookmark mirror's tree log with full names r=kitcambridge

Comment 10

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/30b4097734ce