Closed Bug 462774 Opened 16 years ago Closed 16 years ago

drop JSON.jsm

Categories

(Core :: General, defect)

Product:
Core
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.9.1b2

People

(Reporter: zeniko, Assigned: zeniko)

References

Details

(Keywords: addon-compat, dev-doc-complete, verified1.9.1)

Attachments

(2 files)

remove JSON.jsm
(deleted), patch
Gavin
: review+
sayrer
: review+
brendan
: superreview+
beltzner
: approval1.9.1b2+
Details | Diff | Splinter Review
add JSON.jsm to removed-files.in
(deleted), patch
Details | Diff | Splinter Review
JSON.jsm's export was renamed from JSON to JSONModule in bug 458959. As we've now got native JSON and as consumers of JSON.jsm will have to be updated, anyway, this is the ideal moment to completely drop that module.
Flags: blocking1.9.1?
Attached patch remove JSON.jsm (deleted) — — Splinter Review
This patch removes JSON.jsm and its unit tests. The only remaining caller in our tree(s) will be updated in bug 407110. Brendan: Do I need review for this patch at all?
Attachment #345970 - Flags: superreview?(brendan)
Blocks: 450633
Keywords: dev-doc-needed
Comment on attachment 345970 [details] [diff] [review] remove JSON.jsm Mike: This is pure code removal and would probably be a good idea to be done for Beta 2 so that extension developers don't accidentally updated their code in the wrong way (using JSONModule instead of native JSON).
Attachment #345970 - Flags: review?(shaver)
Attachment #345970 - Flags: superreview?(brendan)
Attachment #345970 - Flags: superreview+
Attachment #345970 - Flags: review?(shaver)
Attachment #345970 - Flags: review?(gavin.sharp)
Comment on attachment 345970 [details] [diff] [review] remove JSON.jsm rs=me, asking sayrer to r+ since his name is most on the hg log (not that that means much! Gavin, feel free to fwd to sayrer, but this looks ok to me and I agree we want it out ASAP). /be
Comment on attachment 345970 [details] [diff] [review] remove JSON.jsm Sounds OK to me, assuming sayrer is on board. We should update https://developer.mozilla.org/en/JSON#Using_JSON.jsm though, and perhaps get Mark to post a followup to http://starkravingfinkle.org/blog/2008/02/extension-developers-native-json-parsing/ .
Attachment #345970 - Flags: review?(gavin.sharp) → review+
Comment on attachment 345970 [details] [diff] [review] remove JSON.jsm Robert: Any objections to getting rid of this legacy code (even though native JSON doesn't allow for blacklists yet)?
Attachment #345970 - Flags: review?(sayrer)
Attachment #345970 - Flags: approval1.9.1b2?
Comment on attachment 345970 [details] [diff] [review] remove JSON.jsm a? after r+ please :)
Attachment #345970 - Flags: approval1.9.1b2?
Attachment #345970 - Flags: review?(sayrer) → review+
Comment on attachment 345970 [details] [diff] [review] remove JSON.jsm I agree with Simon that we should get this in for beta 2 - better sooner than later.
Attachment #345970 - Flags: approval1.9.1b2?
Comment on attachment 345970 [details] [diff] [review] remove JSON.jsm a=beltzner
Attachment #345970 - Flags: approval1.9.1b2? → approval1.9.1b2+
Flags: blocking1.9.1? → blocking1.9.1+
Target Milestone: --- → mozilla1.9.1
Pushed: http://hg.mozilla.org/mozilla-central/rev/ec9a1864d1fb
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
I've backed this out for now as it causes the mac boxes to burn. When re-landing you'll need someone from the build team to clobber some part of the objdir to get rid of the symlink to JSON.jsm
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Don't we have to remove that file from packaging as well (i.e. packages-static and probably even adding to removed-files.in)?
sorry, in packages-static we package modules/* but removed-files.in may still be something we want/need so auto-update removes it.
Good point. Looks like missing files are just ignored there so it didn't break the builds but should be done for completeness.
Possibly when build are cleaning up after this they can also remove the symlinks for PlacesBackground.jsm on the win and linux boxes too *sigh*
Keywords: late-compat
Pushed again: http://hg.mozilla.org/mozilla-central/rev/f901ad15838d
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1 → mozilla1.9.1b2
Blocks: 465358
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090123 Shiretoko/3.1b3pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090123 Minefield/3.2a1pre JSON.jsm is no longer present.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: