Closed Bug 1337188 Opened 7 years ago Closed 7 years ago

`EngineManager::register` shouldn't return a value

Categories

(Firefox :: Sync, defect, P3)

Product:
Firefox
Component:
Sync
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 54
Tracking Flags:
Tracking Status
firefox54 --- fixed

People

(Reporter: lina, Assigned: mayank, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

Bug-1337188.patch
671 bytes, patch
lina
: review+
Details | Diff | Splinter Review 
I noticed while working on bug 1317223 that `EngineManager::register` returns its argument if registration failed, but returns nothing on success (http://searchfox.org/mozilla-central/rev/f5077ad52f8b90183e73038869f6140f0afbf427/services/sync/modules/engines.js#607).

It doesn't look like we rely on the return value anywhere, so it should be safe to remove this line, but filing as a separate bug so that we don't lose track of it. This could be a good first bug.
Mentor: kit
Keywords: good-first-bug
Priority: -- → P3
Hi, I am working on this bug. Attaching patch. Please review.
Thanks
Attachment #8837269 - Flags: review?(kit)
Comment on attachment 8837269 [details] [diff] [review]
Bug-1337188.patch

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

Thanks! Pushed to try here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b130093070f2cb604967e320b0533d55acdb79fa
Attachment #8837269 - Flags: review?(kit) → review+
Try looks good. The xpcshell failures are from bug 694162.
Assignee: nobody → mayanksri18
Status: NEW → ASSIGNED
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee6c8b224924
Remove unused return statement from EngineManager::register. r=kit
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ee6c8b224924
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
You need to log in before you can comment on or make changes to this bug.