Closed Bug 1264491 Opened 4 years ago Closed 4 years ago

Update docs for distribution ID

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: mcomella, Assigned: mkaply)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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)
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.
Attached patch Update ping doc (obsolete) — Splinter Review
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?
> 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 :)
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)
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+
> 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?
https://hg.mozilla.org/integration/fx-team/rev/044cc30c767adab538030de04d5b0406bd1b2891
Bug 1264491 - Update core ping docs for distributionId; r=gfritzsche DONTBUILD
https://hg.mozilla.org/integration/fx-team/rev/69ee7b530ca7c14d896d422f6cf748d192868274
Bug 1264491 - Backout accidental checkins that had nothing to do with bug; DONTBUILD
This did get uplifted, but the bug wasn't closed.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.