Closed
Bug 398810
Opened 17 years ago
Closed 16 years ago
Remove MOZILLA_1_8_BRANCH ifdefs from core on trunk
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.1a1
People
(Reporter: bzbarsky, Assigned: glandium)
Details
Attachments
(2 files, 1 obsolete file)
6.62 KB,
patch
|
sicking
:
review+
brendan
:
review+
benjamin
:
review+
mtschrep
:
approval1.9-
|
Details | Diff | Splinter Review |
4.50 KB,
patch
|
Details | Diff | Splinter Review |
See bug 347910 comment 2.
Reporter | ||
Updated•17 years ago
|
Summary: Remove MOZILLA_1_8_BRANCH interfaces from core on trunk → Remove MOZILLA_1_8_BRANCH ifdefs from core on trunk
Assignee | ||
Comment 2•16 years ago
|
||
I don't know if this belongs here or to bug #398811
Attachment #317963 -
Flags: review?(gavin.sharp)
Comment 3•16 years ago
|
||
Mike, if you wouldn't mind, please check the "Take bug" checkbox when you attach a patch that fixes a bug so that the bug can be assigned to you. :)
Assignee: nobody → mh+mozilla
OS: Linux → All
Hardware: PC → All
Comment 4•16 years ago
|
||
You should get the relevant module owners to sign off on these changes. I'm not sure that all the code in /extensions doesn't still rely on being buildable on trunk or branch, for example. I think most of the core code is probably OK (though it would be good to verify why the ifdefs were added in the first place, and confirm that it was because of the plans to do automatic mirroring).
OS: All → Linux
Hardware: All → PC
Comment 5•16 years ago
|
||
Pretty sure canvas3d is trunk-only, and I know extensions/metrics/ is trunk only.
Assignee | ||
Updated•16 years ago
|
Attachment #317962 -
Flags: review?(jonas)
Attachment #317962 -
Flags: review?(gavin.sharp)
Attachment #317962 -
Flags: review?(brendan)
Assignee | ||
Updated•16 years ago
|
Attachment #317962 -
Flags: review?(benjamin)
Assignee | ||
Comment 6•16 years ago
|
||
(In reply to comment #4) > You should get the relevant module owners to sign off on these changes. I'm not > sure that all the code in /extensions doesn't still rely on being buildable on > trunk or branch, for example. I think most of the core code is probably OK > (though it would be good to verify why the ifdefs were added in the first > place, and confirm that it was because of the plans to do automatic mirroring). Nothing for these extensions on http://www.mozilla.org/owners.html :-/
Comment 7•16 years ago
|
||
Comment on attachment 317962 [details] [diff] [review] patch r=me for the js/src changes. /be
Attachment #317962 -
Flags: review?(brendan) → review+
Updated•16 years ago
|
Attachment #317963 -
Flags: review?(pete)
Updated•16 years ago
|
Attachment #317963 -
Flags: review?(pete) → review+
Updated•16 years ago
|
Attachment #317963 -
Flags: review?(vladimir)
Attachment #317963 -
Flags: review?(mozilla)
Attachment #317963 -
Flags: review?(jonas)
Updated•16 years ago
|
Attachment #317963 -
Flags: review?(jonas)
Attachment #317963 -
Flags: review?(gavin.sharp)
Comment on attachment 317963 [details] [diff] [review] patch for extensions looks fine here, though I plan on cvs rm'ing extensions/canvas3d shortly (moving the repo elsewhere)
Attachment #317963 -
Flags: review?(vladimir) → review+
Comment 9•16 years ago
|
||
Comment on attachment 317963 [details] [diff] [review] patch for extensions Please leave the CCK code in the tree. The CCK, while built on the trunk, can still build CCKs that work in FF 2.
Attachment #317963 -
Flags: review?(mozilla) → review-
Assignee | ||
Comment 10•16 years ago
|
||
Same as previous, without changes to cck, which means it should be review+ for the rest.
Attachment #317963 -
Attachment is obsolete: true
Updated•16 years ago
|
Attachment #317962 -
Flags: review?(benjamin) → review+
Attachment #317962 -
Flags: review?(jonas) → review+
Comment 11•16 years ago
|
||
Any progress on this bug?
Comment 12•16 years ago
|
||
Comment on attachment 317962 [details] [diff] [review] patch Removes unneeded MOZILLA_1_8_BRANCH |#define|s from the trunk.
Attachment #317962 -
Flags: approval1.9?
Comment 13•16 years ago
|
||
Comment on attachment 318188 [details] [diff] [review] patch for extensions v2 Removes unneeded MOZILLA_1_8_BRANCH |#define|s from the trunk.
Attachment #318188 -
Flags: approval1.9?
Comment 14•16 years ago
|
||
Comment on attachment 318188 [details] [diff] [review] patch for extensions v2 Only taking showstopper issues at this point. If this is one of those please re-nom with why.
Attachment #318188 -
Flags: approval1.9? → approval1.9-
Comment 15•16 years ago
|
||
Comment on attachment 317962 [details] [diff] [review] patch Only taking showstopper issues at this point. If this is one of those please re-nom with why.
Attachment #317962 -
Flags: approval1.9? → approval1.9-
Comment 16•16 years ago
|
||
Comment on attachment 318188 [details] [diff] [review] patch for extensions v2 See bug 347910, comment #2. bz _really_ wants this. Patch is very safe, as it's just removing code that was only used on the branch and nothing more.
Attachment #318188 -
Flags: approval1.9- → approval1.9?
Comment 17•16 years ago
|
||
Comment on attachment 317962 [details] [diff] [review] patch See bug 347910, comment #2. bz _really_ wants this. Patch is very safe, as it's just removing code that was only used on the branch and nothing more.
Attachment #317962 -
Flags: approval1.9- → approval1.9?
Comment 18•16 years ago
|
||
One of those zero-risk issues, since we don't use this code at all.
Comment 19•16 years ago
|
||
We should take this to get rid of 1.8 cruft in the hg.mozilla.org and 1.9.x worlds, ASAP. /be
Comment 20•16 years ago
|
||
Comment on attachment 317962 [details] [diff] [review] patch Per discussion on IRC with shaver, gavin, reed, we don't have an urgent need for this on 1.9. Land away on Hg.
Attachment #317962 -
Flags: approval1.9? → approval1.9-
Comment 21•16 years ago
|
||
Comment on attachment 318188 [details] [diff] [review] patch for extensions v2 Actually, this is all NPOTB. I'll just land this (minus the content changes that are really in the other patch).
Attachment #318188 -
Flags: approval1.9?
Comment 22•16 years ago
|
||
Comment on attachment 318188 [details] [diff] [review] patch for extensions v2 Per discussion on IRC with shaver, gavin, reed, we don't have an urgent need for this on 1.9. Land away on Hg.
Attachment #318188 -
Flags: approval1.9-
Updated•16 years ago
|
Keywords: checkin-needed
Comment 23•16 years ago
|
||
Comment on attachment 318188 [details] [diff] [review] patch for extensions v2 NPOTB (minus the content/ changes that are also in the other patch).
Attachment #318188 -
Flags: approval1.9-
We have tons of dead code in the tree, and shouldn't be injecting noise into the end-game by landing patches that are _designed_to_do_nothing_. The _interfaces_ are already gone, which is the most important issue as per bz's original bug, and so don't represent an attractive nuisance for code consumers. In hg, let's by all means remove dead code for 1.9.x and Mozilla 2. This is just the tip of the iceberg, I'm sure. http://mxr.mozilla.org/firefox/search?string=MWContext !
Comment 25•16 years ago
|
||
Checking in extensions/metrics/build/nsMetricsModule.cpp; /cvsroot/mozilla/extensions/metrics/build/nsMetricsModule.cpp,v <-- nsMetricsModule.cpp new revision: 1.5; previous revision: 1.4 done Checking in extensions/metrics/src/nsLoadCollector.cpp; /cvsroot/mozilla/extensions/metrics/src/nsLoadCollector.cpp,v <-- nsLoadCollector.cpp new revision: 1.25; previous revision: 1.24 done Checking in extensions/metrics/src/nsMetricsService.cpp; /cvsroot/mozilla/extensions/metrics/src/nsMetricsService.cpp,v <-- nsMetricsService.cpp new revision: 1.46; previous revision: 1.45 done Checking in extensions/metrics/test/TestMetricsConfig.cpp; /cvsroot/mozilla/extensions/metrics/test/TestMetricsConfig.cpp,v <-- TestMetricsConfig.cpp new revision: 1.4; previous revision: 1.3 done
Comment 26•16 years ago
|
||
Pushed in 15888:5fba7bea3723.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing after 1.9]
Target Milestone: --- → mozilla1.9.1a1
You need to log in
before you can comment on or make changes to this bug.
Description
•