Update docs for distribution ID

RESOLVED FIXED

Status

()

Firefox for Android
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mcomella, Assigned: mkaply)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

2 years ago
Mike, can you add the distribution ID in as optional?

Don't forget to update the version history at the bottom.

The file is here: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/docs/core-ping.rst
Flags: needinfo?(mozilla)
Blocks: 1251636
(Assignee)

Comment 1

2 years ago
I assume I can check this in without approval because it is not part of the build?

Do I check it directly into mozilla-central?
This needs review too, it's part of the code base.
After that it also lands into m-c.
(Reporter)

Updated

2 years ago
Blocks: 1266554
(Assignee)

Comment 3

2 years ago
Created attachment 8744346 [details] [diff] [review]
Update ping doc

Here are the updates.
Flags: needinfo?(mozilla)
Attachment #8744346 - Flags: review?(michael.l.comella)
Comment on attachment 8744346 [details] [diff] [review]
Update ping doc

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

I think you didn't intend to submit the .xml changes with this patch?

::: toolkit/components/telemetry/docs/core-ping.rst
@@ +61,5 @@
> +~~~~~~
> +The ``distributionId`` contains the distribution ID as specified by
> +preferences.json for a given distribution. More information on distributions
> +can be found here:
> +https://wiki.mozilla.org/Mobile/Distribution_Files

rst documents can do proper links, see e.g. the examples here:
https://dxr.mozilla.org/mozilla-central/rev/0891f0fa044cba28024849803e170ed7700e01e0/toolkit/components/telemetry/docs/main-ping.rst#96

You can build the documentation using |mach doc|, it will print where to find the generated html files after building completes.

@@ +109,5 @@
>  Version history
>  ---------------
> +* v4: ``profileDate`` will return package install time when times.json is not available
> +* v3: added ``defaultSearch``
> +* v2: added ``distributionId``

Michael, i assume its probably easiest if we sort the versioning situation in a separate bug?
(Assignee)

Comment 5

2 years ago
> I think you didn't intend to submit the .xml changes with this patch?

Doh. I'll fix.

> You can build the documentation using |mach doc|, it will print where to find the generated html files after building completes.

Thanks. I had no idea what kind of documentation I was building :)
(Reporter)

Comment 6

2 years ago
Comment on attachment 8744346 [details] [diff] [review]
Update ping doc

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

(In reply to Georg Fritzsche [:gfritzsche] from comment #4)
> Michael, i assume its probably easiest if we sort the versioning situation
> in a separate bug?

Yeah, I've filed bug 1266554 to handle this. Sorry for not being explicit!

Mike, you can just send the next review round to Georg.
Attachment #8744346 - Flags: review?(michael.l.comella)
(Assignee)

Comment 7

2 years ago
Created attachment 8744972 [details] [diff] [review]
Update patch per comments

Fixed link per your comments.

I wasn't sure how to update the version section, so I just added the distribution comment onto v2.

I assume we'll clean this up in bug 1266554
Attachment #8744346 - Attachment is obsolete: true
Attachment #8744972 - Flags: review?(gfritzsche)
Comment on attachment 8744972 [details] [diff] [review]
Update patch per comments

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

::: toolkit/components/telemetry/docs/core-ping.rst
@@ +108,5 @@
>  
>  Version history
>  ---------------
>  * v3: ``profileDate`` will return package install time when times.json is not available
> +* v2: added ``defaultSearch``, added ``distributionId``

Lets just add a "v4" line for distributionId (i.e. lets keep with showing how things landed on Nightly).
Then we can fix this properly in the other bug to match what we have on release.
Attachment #8744972 - Flags: review?(gfritzsche) → review+
(Assignee)

Comment 9

2 years ago
> Lets just add a "v4" line for distributionId (i.e. lets keep with showing how things landed on Nightly).
Then we can fix this properly in the other bug to match what we have on release.

Sorry, I was thinking that that v had to match the ping version.

Doh.

Does this go through fx-team/inbound or direct to central as NPOB?
(Assignee)

Comment 10

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/044cc30c767adab538030de04d5b0406bd1b2891
Bug 1264491 - Update core ping docs for distributionId; r=gfritzsche DONTBUILD
(Assignee)

Comment 11

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/69ee7b530ca7c14d896d422f6cf748d192868274
Bug 1264491 - Backout accidental checkins that had nothing to do with bug; DONTBUILD
(Assignee)

Comment 12

2 years ago
This did get uplifted, but the bug wasn't closed.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.