Closed Bug 870925 Opened 7 years ago Closed 6 years ago

Provider: submissions

Categories

(Firefox Health Report Graveyard :: Client: Android, defect, P2)

ARM
Android
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rnewman, Assigned: mcomella)

References

Details

Attachments

(1 file)

This is the Android equivalent of Bug 854018.

We'll want to track upload attempts, successes, and failures.

Note that this needs to compute environments outside of Fennec, so I might have to rejig some stuff.
OS: Mac OS X → Android
Priority: -- → P2
Hardware: x86 → ARM
Depends on: 873360
Depends on: 828654
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
^ needinfo is for feedback. Specifically, I would like to know if overriding the RequestDelegate to record submissions is the correct course of action for implementing a patch.

Otherwise, the feedback does not need to be very thorough and can wait for the review.
LGTM.
Flags: needinfo?(rnewman)
Drat, wrong bug. Still reviewing this one; Bug 893910 looks good to me :D
r? - part 8 - getCacheSuffix changes as spoken about on IRC. I can move it to part 0 if you'd prefer.
Flags: needinfo?(nalexander)
r? - parts 1-14, sans part 8 and its review comment:

https://github.com/mozilla-services/android-sync/pull/332

Note that upload(...) could be tested better by ensuring the Delegate passed to uploadPayload's handle* methods are called in various error cases, however, that seems like a lot of extra effort for little gain.
Flags: needinfo?(rnewman)
(In reply to Michael Comella (:mcomella) from comment #5)
> r? - part 8 - getCacheSuffix changes as spoken about on IRC. I can move it
> to part 0 if you'd prefer.

Sorry, this slipped -- lgtm.
Flags: needinfo?(nalexander)
Have a structural comment on the PR. Otherwise looking good, I think.
Flags: needinfo?(rnewman)
Attachment #819855 - Attachment description: Github PR → Add submissions provider
Comment on attachment 819855 [details] [review]
Add submissions provider

I made the `SubmissionsStatusCounter` changes you recommended (but yet sans testing). Is that what you were looking for?
Attachment #819855 - Flags: feedback?(rnewman)
Comment on attachment 819855 [details] [review]
Add submissions provider

Yeah, that's it!
Attachment #819855 - Flags: feedback?(rnewman) → feedback+
Comment on attachment 819855 [details] [review]
Add submissions provider

I added the tests to the refactor.
Attachment #819855 - Flags: feedback+ → review?(rnewman)
Attachment #819855 - Flags: review?(rnewman) → review+
https://hg.mozilla.org/mozilla-central/rev/469b3bd4a91d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.