Closed Bug 645049 Opened 9 years ago Closed 2 years ago
Remove [deprecated] methods on moz
IStorage Base Statement .idl
26.11 KB, patch
|Details | Diff | Splinter Review|
5.70 KB, patch
|Details | Diff | Splinter Review|
59 bytes, text/x-review-board-request
There aren't many uses left in the tree. This has been marked as deprecated for a while now, so it's time to nuke them.
This updates all the tests to use the newer API. Going to push this all to try at the same time, but things passed locally. I probably don't need review for this change, but I figure it doesn't hurt to ask.
Attachment #521862 - Flags: review?(dtownsend)
These were actually marked as deprecated in bug 507414 which landed in 1.9.3a3. We have only shipped one major version with the new API, but I still think we should remove the old one.
All dependent bugs now have patches up. When those all land, we can actually do this!
(In reply to comment #5) > Will this resolve bug 589032 No, that's different. > and bug 589035? Bug 645094 does.
Whiteboard: [needs review asuth][needs review Mossop][needs sr rstrong]
Comment on attachment 521862 [details] [diff] [review] Fix tests v1.0 (checked in) Have you done a search on AMO to figure out how many add-ons will get hurt by this?
Attachment #521862 - Flags: review?(dtownsend) → review+
(In reply to comment #7) > Have you done a search on AMO to figure out how many add-ons will get hurt by > this? bindUTF8StringParameter: Too many hits, displaying the first 1000 bindStringParameter: Found 876 matching lines in 138 files bindDoubleParameter: Found 81 matching lines in 34 files bindInt32Parameter: Too many hits, displaying the first 1000 bindInt64Parameter: Found 767 matching lines in 229 files bindNullParameter: Found 83 matching lines in 43 files bindBlobParameter: Found 115 matching lines in 113 files UTF8String and Int32 parameter are clearly used a lot. I'm fine with waiting until Firefox 6 to land these changes, but not any longer. The JS helper (without creating an explicit wrapper) has been available since 1.9.1 (Firefox 3.5) which allows JS consumers to just do stmt.params[index]. I would prefer to land this for Firefox 5, however.
Whiteboard: [needs review asuth][needs review Mossop][needs sr rstrong] → [needs review asuth][needs sr rstrong]
Comment on attachment 521930 [details] [diff] [review] Drop Methods v1.0 Looks like this will cause a lot of extension breakage so without an overriding reason or mitigation of this breakage in place both along with driver understanding this and approving I don't think this should go in for Firefox 5. Looks fine otherwise
Attachment #521930 - Flags: superreview?(robert.bugzilla) → superreview+
Whiteboard: [needs review asuth][needs sr rstrong] → [needs review asuth]
Comment on attachment 521930 [details] [diff] [review] Drop Methods v1.0 Looks technically fine to me. How do you search extension code on AMO? That sounds like a neat trick I would like to be able to do.
Attachment #521930 - Flags: review?(bugmail) → review+
(In reply to comment #10) > How do you search extension code on AMO? That sounds like a neat trick I would > like to be able to do. https://mxr.mozilla.org/addons/ but it does require LDAP authentication.
Whiteboard: [needs review asuth] → [land for Firefox 6]
Comment on attachment 521862 [details] [diff] [review] Fix tests v1.0 (checked in) http://hg.mozilla.org/mozilla-central/rev/046c166f999f
Attachment #521862 - Attachment description: Fix tests v1.0 → Fix tests v1.0 (checked in)
Not comfortable landing this so late in Firefox 6's cycle. This will happen for 7.
Whiteboard: [land for Firefox 6] → [land for Firefox 7]
This appears to have slipped the Firefox 7 date by a little bit. Is this still something that should be landed?
Whoops! Guess that whiteboard field did not have magic powers after all. This may need to be re-audited and such. :mak is owner now, switching over to him.
Flags: needinfo?(bugmail) → needinfo?(mak77)
We need updated stats to be able to decide how to move. Surely we still would like to do this, but we might have to reach most add-ons by mail through the AMO team first. From what I see in https://mxr.mozilla.org/addons/search?string=bind%23Parameter\(®exp=on there are still thousands of consumers, we should try to reduce those, or this would break too many add-ons. Something we can do in the meanwhile is to fix in-tree usage http://mxr.mozilla.org/mozilla-central/search?string=bind%23Parameter\(®exp=1 this part can be a simple mentored task. Also the mdn documentation should be fixed, if we still suggest using those methods in the docs, it's unlikely add-ons may stop using them...
Whiteboard: [land for Firefox 7] → [good first bug][mentor=mak][lang=js]
the first mentored things to do here are file a dependency to fix in-tree usage (and make a patch for that) and update mdn examples/docs.
Assignee: sdwilsh → nobody
Status: ASSIGNED → NEW
Are we showing console warnings when those methods are used? That would also help push add-on devs to look for alternatives.
nope, I'm going to file a bug for that.
So now we have a dependency to warn and one to remove in-tree consumers. both are mentored. here we have basically done patches and to update the docs, only the latter is actionable atm.
Whiteboard: [good first bug][mentor=mak][lang=js]
we can do this from FF57 on.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Comment on attachment 8894138 [details] Bug 645049 - Remove [deprecated] methods on mozIStorageBaseStatement.idl. https://reviewboard.mozilla.org/r/165194/#review170578 It's great to have this done! Thank you!
Attachment #8894138 - Flags: review?(bugmail) → review+
Pushed by VYV03354@nifty.ne.jp: https://hg.mozilla.org/integration/autoland/rev/46153627ee5f Remove [deprecated] methods on mozIStorageBaseStatement.idl. r=asuth
You need to log in before you can comment on or make changes to this bug.