398810 - Remove MOZILLA_1_8_BRANCH ifdefs from core on trunk

Status: RESOLVED FIXED

Description

[Boris Zbarsky [:bz] (no decent commit message means r-)]

See bug 347910 comment 2.

Comment 1

[Mike Hommey [:glandium]]

Created attachment 317962 [details] [diff] [review]
patch

This should do

Comment 2

[Mike Hommey [:glandium]]

Created attachment 317963 [details] [diff] [review]
patch for extensions

I don't know if this belongs here or to bug #398811

Comment 3

[Reed Loden [:reed] (use needinfo?)]

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. :)

Comment 4

[:Gavin Sharp [email: [e-mail]]]

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).

Comment 5

[Reed Loden [:reed] (use needinfo?)]

Pretty sure canvas3d is trunk-only, and I know extensions/metrics/ is trunk only.

Comment 6

[Mike Hommey [:glandium]]

(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

[Brendan Eich [:brendan]]

Comment on attachment 317962 [details] [diff] [review]
patch

r=me for the js/src changes.

/be

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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)

Comment 9

[Mike Kaply [:mkaply]]

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.

Comment 10

[Mike Hommey [:glandium]]

Created attachment 318188 [details] [diff] [review]
patch for extensions v2

Same as previous, without changes to cck, which means it should be review+ for the rest.

Comment 11

[Tomas]

Any progress on this bug?

Comment 12

[Reed Loden [:reed] (use needinfo?)]

Comment on attachment 317962 [details] [diff] [review]
patch

Removes unneeded MOZILLA_1_8_BRANCH |#define|s from the trunk.

Comment 13

[Reed Loden [:reed] (use needinfo?)]

Comment on attachment 318188 [details] [diff] [review]
patch for extensions v2

Removes unneeded MOZILLA_1_8_BRANCH |#define|s from the trunk.

Comment 14

[Mike Schroepfer]

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.

Comment 15

[Mike Schroepfer]

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.

Comment 16

[Reed Loden [:reed] (use needinfo?)]

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.

Comment 17

[Reed Loden [:reed] (use needinfo?)]

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.

Comment 18

[Reed Loden [:reed] (use needinfo?)]

One of those zero-risk issues, since we don't use this code at all.

Comment 19

[Brendan Eich [:brendan]]

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

[Mike Schroepfer]

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.

Comment 21

[Reed Loden [:reed] (use needinfo?)]

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).

Comment 22

[Mike Schroepfer]

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.

Comment 23

[Reed Loden [:reed] (use needinfo?)]

Comment on attachment 318188 [details] [diff] [review]
patch for extensions v2

NPOTB (minus the content/ changes that are also in the other patch).

Comment 24

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

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

[Reed Loden [:reed] (use needinfo?)]

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

[Reed Loden [:reed] (use needinfo?)]

Pushed in 15888:5fba7bea3723.