Last Comment Bug 523820 - Remove old MOZILLA_1_9_1_BRANCH ifdefs from comm-central code
: Remove old MOZILLA_1_9_1_BRANCH ifdefs from comm-central code
Status: RESOLVED FIXED
: fixed-seamonkey2.0.1
Product: MailNews Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 3.1a1
Assigned To: Serge Gautherie (:sgautherie)
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
: 516452 (view as bug list)
Depends on: 255503 504029 521293 521624 522211 522713 525331 530010 531283 532573 533627 541125
Blocks: 514665
  Show dependency treegraph
 
Reported: 2009-10-22 06:35 PDT by Mark Banner (:standard8) (afk until 26th July)
Modified: 2010-05-30 03:04 PDT (History)
6 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
(Av1-SM) Remove obsolete cases (31.61 KB, patch)
2009-10-23 19:32 PDT, Serge Gautherie (:sgautherie)
no flags Details | Diff | Splinter Review
(Av1a-SM) Remove obsolete cases [Checkin: Comment 5] (31.68 KB, patch)
2009-10-26 09:19 PDT, Serge Gautherie (:sgautherie)
kairo: review+
Details | Diff | Splinter Review
(Bv1-TB) Clean /mail/installer/* up, Improve sort order, Remove obsolete jsctypes.xpt. (26.78 KB, patch)
2009-11-13 00:53 PST, Serge Gautherie (:sgautherie)
standard8: review-
Details | Diff | Splinter Review
(Cv1) Clean /mailnews/* up [Checkin: See comment 11+18] (6.98 KB, patch)
2009-11-13 01:11 PST, Serge Gautherie (:sgautherie)
standard8: review+
standard8: superreview+
Details | Diff | Splinter Review
(Dv1) Clean "configure.in" up, the obvious part [Checkin: Comment 12] (8.49 KB, patch)
2009-11-13 01:32 PST, Serge Gautherie (:sgautherie)
standard8: review+
Details | Diff | Splinter Review
(Ev1-SM-191) Clean /suite/installer/* up [Checkin: Comment 22] (22.24 KB, patch)
2009-11-13 07:36 PST, Serge Gautherie (:sgautherie)
kairo: review+
kairo: approval‑seamonkey2.0.1+
Details | Diff | Splinter Review
(Fv1-SM) Remove/Update missed cases (4.41 KB, patch)
2009-11-26 10:08 PST, Serge Gautherie (:sgautherie)
kairo: review-
Details | Diff | Splinter Review
(Fv2-SM) Remove/Update missed cases (4.35 KB, patch)
2009-12-13 10:21 PST, Serge Gautherie (:sgautherie)
no flags Details | Diff | Splinter Review
(Gv1) Clean 'configure.in' up, the remaining part [Checkin: Comment 35] (5.03 KB, patch)
2009-12-17 16:00 PST, Serge Gautherie (:sgautherie)
bugspam.Callek: review+
Details | Diff | Splinter Review
(Hv1-SM) Remove/Update (2 of 3) missed cases, Use waitForFocus() in 2 tests. (4.21 KB, patch)
2009-12-20 09:20 PST, Serge Gautherie (:sgautherie)
no flags Details | Diff | Splinter Review
(Hv1a-SM) Remove/Update (2 of 3) missed cases, Use waitForFocus() [Checkin: Comment 30] (3.36 KB, patch)
2009-12-20 16:13 PST, Serge Gautherie (:sgautherie)
neil: review+
Details | Diff | Splinter Review
(Fv2a-SM) Remove last missed case [Checkin: Comment 37] (1.31 KB, patch)
2009-12-20 17:34 PST, Serge Gautherie (:sgautherie)
bugspam.Callek: review+
Details | Diff | Splinter Review

Description Mark Banner (:standard8) (afk until 26th July) 2009-10-22 06:35:01 PDT
Now that we have branched, we should remove the MOZILLA_1_9_1_BRANCH ifdefs from comm-central.

However in the interests of code stability for developers, I'd like to limit what we do until after Thunderbird has gone into its code freeze for rc1. Hence, mailnews/ and mail/ should be largely left alone until after code freeze unless there's specific tidy up we want to do for other reasons.
Comment 1 Serge Gautherie (:sgautherie) 2009-10-23 19:32:34 PDT
Created attachment 408165 [details] [diff] [review]
(Av1-SM) Remove obsolete cases
Comment 2 neil@parkwaycc.co.uk 2009-10-25 15:01:37 PDT
Comment on attachment 408165 [details] [diff] [review]
(Av1-SM) Remove obsolete cases

>-# We need to do this before we include config.mk and thus app-config.mk
>+# before we include config.mk and thus app-config.mk.
This comment change is wrong. DEFINES needs to be set before config.mk is included, because app-config.mk is loaded after config.mk reads DEFINES.

At least, I think that's why I had to do it this way to get it to compile.
Comment 3 Serge Gautherie (:sgautherie) 2009-10-26 09:19:32 PDT
Created attachment 408401 [details] [diff] [review]
(Av1a-SM) Remove obsolete cases
[Checkin: Comment 5]

Av1-SM, with comment 2 suggestion(s).
Comment 4 Robert Kaiser 2009-11-02 07:46:56 PST
Comment on attachment 408401 [details] [diff] [review]
(Av1a-SM) Remove obsolete cases
[Checkin: Comment 5]

>-#ifdef MOZILLA_1_9_1_BRANCH
>-bin/components/liboji.so
>-#endif

Could you check if this really needs to be removed or changed into a 1_9_2 ifdef? I heard that possibly they have re-enabled OJI for 1.9.2 only.
Of course, the same applies for any other places in the bug that mention "oji" somehow.

r=me with that taken in mind.
Comment 5 Serge Gautherie (:sgautherie) 2009-11-02 15:24:47 PST
Comment on attachment 408401 [details] [diff] [review]
(Av1a-SM) Remove obsolete cases
[Checkin: Comment 5]


http://hg.mozilla.org/comm-central/rev/955b4e732760


(In reply to comment #4)
> they have re-enabled OJI for 1.9.2 only.

Yes, but on MacOSX only: see bug 521624.
Comment 6 Serge Gautherie (:sgautherie) 2009-11-13 00:53:36 PST
Created attachment 412156 [details] [diff] [review]
(Bv1-TB) Clean /mail/installer/* up, Improve sort order, Remove obsolete jsctypes.xpt.
Comment 7 Serge Gautherie (:sgautherie) 2009-11-13 00:58:47 PST
Comment on attachment 412156 [details] [diff] [review]
(Bv1-TB) Clean /mail/installer/* up, Improve sort order, Remove obsolete jsctypes.xpt.

Bug 521293 will add some m-1.9.2 support (back)...
Comment 8 Serge Gautherie (:sgautherie) 2009-11-13 01:11:49 PST
Created attachment 412162 [details] [diff] [review]
(Cv1) Clean /mailnews/* up
[Checkin: See comment 11+18]
Comment 9 Serge Gautherie (:sgautherie) 2009-11-13 01:32:47 PST
Created attachment 412164 [details] [diff] [review]
(Dv1) Clean "configure.in" up, the obvious part
[Checkin: Comment 12]
Comment 10 Serge Gautherie (:sgautherie) 2009-11-13 07:36:12 PST
Created attachment 412218 [details] [diff] [review]
(Ev1-SM-191) Clean /suite/installer/* up
[Checkin: Comment 22]

We still have packaging issues on SM 2.0.x: this should make life easier in future.
Comment 11 Serge Gautherie (:sgautherie) 2009-11-19 05:28:32 PST
Comment on attachment 412162 [details] [diff] [review]
(Cv1) Clean /mailnews/* up
[Checkin: See comment 11+18]


http://hg.mozilla.org/comm-central/rev/776a79a690ea
Comment 12 Serge Gautherie (:sgautherie) 2009-11-22 14:24:47 PST
Comment on attachment 412164 [details] [diff] [review]
(Dv1) Clean "configure.in" up, the obvious part
[Checkin: Comment 12]


http://hg.mozilla.org/comm-central/rev/6c647a81c780
Comment 13 Ben Bucksch (:BenB) 2009-11-23 18:26:33 PST
mailnews/imap/src/nsImapMailFolder.cpp
-  // Note: make cast in fillArrayWithKeys() match

Please leave this in, thanks.
Comment 14 Serge Gautherie (:sgautherie) 2009-11-24 06:04:24 PST
(In reply to comment #13)
> mailnews/imap/src/nsImapMailFolder.cpp
> -  // Note: make cast in fillArrayWithKeys() match
> 
> Please leave this in, thanks.

You added this there in bug 522342.
Why should I/we keep a m-1.9.1 only comment ?
Comment 15 Ben Bucksch (:BenB) 2009-11-24 06:11:57 PST
> Why should I/we keep a m-1.9.1 only comment

It wasn't a m-1.9.1 only comment, it applied to both and still applies, that's fairly obvious, I just accidentally put it inside the ifdef. The comment is important because I indeed forgot that when adapting to trunk and it caused a crash.
Comment 16 Serge Gautherie (:sgautherie) 2009-11-24 06:45:08 PST
(In reply to comment #15)

Then feel free to just re-add the right comment in the right place. (You know better than I do.)
Comment 17 Ben Bucksch (:BenB) 2009-11-24 06:47:23 PST
I don't want to go through all the reviews. You removed it, you re-add it, please.
Add it exactly where it was, above
  nsTArray<nsCString>* resultArray = new nsTArray<nsCString>;
Comment 18 Ben Bucksch (:BenB) 2009-11-24 10:10:38 PST
Checked in, without reviews, because it's just restoring old code.
<http://hg.mozilla.org/comm-central/rev/d6dab51ecb69>
Comment 19 Serge Gautherie (:sgautherie) 2009-11-26 10:08:07 PST
Created attachment 414749 [details] [diff] [review]
(Fv1-SM) Remove/Update missed cases
Comment 20 Mark Banner (:standard8) (afk until 26th July) 2009-11-26 11:21:55 PST
Comment on attachment 412156 [details] [diff] [review]
(Bv1-TB) Clean /mail/installer/* up, Improve sort order, Remove obsolete jsctypes.xpt.

Sorry, but bug 530010 does this in a cleaner way and makes it work for 1.9.2 as well (unfortunately we were missing some 1.9.2 ifdefs).
Comment 21 Robert Kaiser 2009-12-02 09:11:36 PST
Comment on attachment 414749 [details] [diff] [review]
(Fv1-SM) Remove/Update missed cases

>diff --git a/suite/config/version-191.txt b/suite/config/version-192.txt
>rename from suite/config/version-191.txt
>rename to suite/config/version-192.txt
>--- a/suite/config/version-191.txt
>+++ b/suite/config/version-192.txt
>@@ -1,1 +1,1 @@
>-2.0.1pre
>\ No newline at end of file
>+2.1a1pre
>\ No newline at end of file

I don't really agree with that, as we have no decision if we even target 1.9.2 with anything, and if we set version.txt and the branch one to the same value, we can do away with the distinction altogether, doesn't make any difference.
Comment 22 Serge Gautherie (:sgautherie) 2009-12-02 10:16:50 PST
Comment on attachment 412218 [details] [diff] [review]
(Ev1-SM-191) Clean /suite/installer/* up
[Checkin: Comment 22]


http://hg.mozilla.org/releases/comm-1.9.1/rev/7698a02571a5
Comment 23 Serge Gautherie (:sgautherie) 2009-12-02 10:38:01 PST
(In reply to comment #21)
> (From update of attachment 414749 [details] [diff] [review])
> >diff --git a/suite/config/version-191.txt b/suite/config/version-192.txt
> 
> I don't really agree with that, as we have no decision if we even target 1.9.2
> with anything, and if we set version.txt and the branch one to the same value,

Well, "2.1a1pre" is what one currently gets when building with m-1.9.2: I'm not changing the value, just updating the code.

> we can do away with the distinction altogether, doesn't make any difference.

I thought the "rv:1.9.2xxx" part would be enough ftb,
yet I fully agree that we could use a different value: "2.1a0pre"?!

The point is: unless we would error out when building on m-1.9.2, we have to use some placeholder value :-|
(I know what the situation is wrt SM2.x/m-1.9.2: I just try to keep things open until a decision is made...)
Comment 24 Serge Gautherie (:sgautherie) 2009-12-02 18:34:45 PST
Bug 530010 is doing TB part...
I filed bug 532573 about "Calendar".
Comment 25 Serge Gautherie (:sgautherie) 2009-12-13 10:21:28 PST
Created attachment 417363 [details] [diff] [review]
(Fv2-SM) Remove/Update missed cases

Fv1-SM, with comment 21 suggestion(s).

Per your irc reply to my comment 23 question, this reverts confvars.sh to how it was before bug 502033: no more 1.9.1 support and no 1.9.2 support yet :-|
Comment 26 Serge Gautherie (:sgautherie) 2009-12-17 16:00:59 PST
Created attachment 418290 [details] [diff] [review]
(Gv1) Clean 'configure.in' up, the remaining part
[Checkin: Comment 35]
Comment 27 Serge Gautherie (:sgautherie) 2009-12-20 09:20:51 PST
Created attachment 418554 [details] [diff] [review]
(Hv1-SM) Remove/Update (2 of 3) missed cases, Use waitForFocus() in 2 tests.

waitForFocus(): I'm not fixing failures, I'm just being more safe/explicit.
Comment 28 neil@parkwaycc.co.uk 2009-12-20 13:41:29 PST
Comment on attachment 418554 [details] [diff] [review]
(Hv1-SM) Remove/Update (2 of 3) missed cases, Use waitForFocus() in 2 tests.

The second use of waitForFocus looks entirely reasonable to me but I don't understand the first one; is there a similar patch for Firefox?
Comment 29 Serge Gautherie (:sgautherie) 2009-12-20 16:13:54 PST
Created attachment 418575 [details] [diff] [review]
(Hv1a-SM) Remove/Update (2 of 3) missed cases, Use waitForFocus()
[Checkin: Comment 30]

Hv1-SM, with comment 28 suggestion(s).

(In reply to comment #28)
> I don't understand the first one; is there a similar patch for Firefox?

None that I am aware of and that was just a suggestion: dropping it.
Comment 30 Serge Gautherie (:sgautherie) 2009-12-20 17:30:30 PST
Comment on attachment 418575 [details] [diff] [review]
(Hv1a-SM) Remove/Update (2 of 3) missed cases, Use waitForFocus()
[Checkin: Comment 30]


http://hg.mozilla.org/comm-central/rev/af645db56e86
Comment 31 Serge Gautherie (:sgautherie) 2009-12-20 17:34:05 PST
Created attachment 418580 [details] [diff] [review]
(Fv2a-SM) Remove last missed case
[Checkin: Comment 37]
Comment 32 Robert Kaiser 2010-01-02 04:02:50 PST
Callek, the patch you talked about on IRC yesterday is part of "Gv1" here.
Comment 33 Justin Wood (:Callek) 2010-01-02 19:31:17 PST
Comment on attachment 418290 [details] [diff] [review]
(Gv1) Clean 'configure.in' up, the remaining part
[Checkin: Comment 35]

r+ on this... I don't think we really need sr here, but I did not clear request (been away a while so still catching up)
Comment 34 Serge Gautherie (:sgautherie) 2010-01-03 03:57:37 PST
Comment on attachment 418290 [details] [diff] [review]
(Gv1) Clean 'configure.in' up, the remaining part
[Checkin: Comment 35]


http://hg.mozilla.org/comm-central/rev/2303689fabf4
Comment 35 Serge Gautherie (:sgautherie) 2010-01-03 03:58:31 PST
Comment on attachment 418290 [details] [diff] [review]
(Gv1) Clean 'configure.in' up, the remaining part
[Checkin: Comment 35]


(In reply to comment #34)
> http://hg.mozilla.org/comm-central/rev/2303689fabf4

http://hg.mozilla.org/comm-central/rev/d8a60bb40b9b !
Comment 36 Justin Wood (:Callek) 2010-01-03 17:20:55 PST
Comment on attachment 418580 [details] [diff] [review]
(Fv2a-SM) Remove last missed case
[Checkin: Comment 37]

was just looking at this too!
Comment 37 Serge Gautherie (:sgautherie) 2010-01-03 23:56:49 PST
Comment on attachment 418580 [details] [diff] [review]
(Fv2a-SM) Remove last missed case
[Checkin: Comment 37]


http://hg.mozilla.org/comm-central/rev/fb6a673527df
Comment 38 Robert Kaiser 2010-01-04 08:10:30 PST
(In reply to comment #36)
> (From update of attachment 418580 [details] [diff] [review])
> was just looking at this too!

GRRRRRRRR!!!!!!!!!!!!!!!!!!!

Now we probably need to reintruduce this again if we need to ship from Lorentz!

This was why I held back this review :(
Comment 39 Serge Gautherie (:sgautherie) 2010-02-04 06:09:47 PST
*** Bug 516452 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.