Closed Bug 1308600 Opened 3 years ago Closed 3 years ago
Ready, Get Current Position Ready, and Watch Position Ready methods from Geolocation
Geolocation::ServiceReady is no longer used by any code, so we should remove it. That means that GetCurrentPositionReady and WatchPositionReady only have one caller each, so we should merge each of those methods with GetCurrentPosition and WatchPosition. This will simplify the code in the file and make it easier to reason about. Code: dom/geolocation/Geolocation.cpp, dom/geolocation/Geolocation.h No need to run any tests; if the changes compile, then it's good enough to submit a patch for review.
i a beginner and unable to find code , could you please specify where is dom/geolocation directory
Attached patch for the suggested changes.
Comment on attachment 8799149 [details] [diff] [review] Removed ServiceReady(), Merged *Ready methods into GetCurrentPosition, WatchPosition Review of attachment 8799149 [details] [diff] [review]: ----------------------------------------------------------------- This looks great! We'll need to add a commit message like "Bug 1308600 - Remove unused ServiceReady method from Geolocation. r=jdm" to the patch as well.
Attachment #8799149 - Flags: review+
Hi Josh, I added the commit message (from instructions here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch).
Thanks! Do you have access to our tryserver (https://wiki.mozilla.org/ReleaseEngineering/TryServer)? If not, please apply for level 1 access by following the steps at https://www.mozilla.org/hacking/committer/ ; I will vouch for you. This will allow you to ensure that your changes build on the major supported platforms (windows, linux, mac).
Thanks for help, Josh. I am continuing with the ticket, I (hopefully) got a correct setup of the accounts to be able to push to try. This will be my next step.
I let the tryserver to build with my patch. https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f953e9e3ef2 Josh, what should be the next step?
Looks good! Go ahead and add the `checkin-needed` keyword to the Keywords field in this bug, and then the patch will be automatically merged for you.
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4c784f8b12ed Remove unused ServiceReady method from Geolocation. r=jdm
You need to log in before you can comment on or make changes to this bug.