695255 - Replace uses of NS_ARRAY_LENGTH with mozilla::ArrayLength and mozilla::ArrayEnd in MailNews

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Comment 1

[Philip Chee]

q.v.
Bug 693469:  Implement mozilla::ArrayLength and mozilla::ArrayEnd, and replace uses of NS_ARRAY_LENGTH whenever possible.  (Exceptions: assigning to static initializers, use in static assertions, as template parameters, etc.  These will go away when the relevant compilers have C++11 constexpr support.)

Comment 2

[daletron]

Hi, me and a couple of students would like to take on this bug.  Can we be assigned to this?

Comment 3

[Philip Chee]

Sure just go for it. You should also read the following if you haven't already done so:
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
https://developer.mozilla.org/En/Developer_Guide/How_to_Submit_a_Patch

Please visit us on IRC if you have any questions:
irc://moznet/seamonkey for SeaMonkey specific issues and also anything you want to discuss or need advice on.
irc://moznet/introduction for help with your first patch.

Comment 4

[daletron]

Attachment: a patch that attempts to fix the bug

Comment 5

[daletron]

Along with the patch, I also made changes to 

comm-central\mozilla\accessible\src\base\nsARIAMap.cpp
&
comm-central\mozilla\dom\system\gonk\SystemWorkerManager.cpp

but for some reason can't get the patch to recognize the changes no matter what i do.  "hg diff" never recognizes it.

Comment 6

[Philip Chee]

> "hg diff" never recognizes it.
Try "hg qrefresh"

Comment 7

[daletron]

hey Philip, ya I been doing the "hg qrefresh" everytime i wanted to push a change into my patch.  (and hg diff to make sure they recognize my change)

i was talking to some people in #introduction about it, and one person said according to this bugzilla report that he doesn't think i need or should change those files, and another says it might be because the files aren't technically in the comm-central repository?  or that they're a sub-repo from mozilla-central?

Comment 8

[Tony Mechelynck [:tonymec]]

In reply to comment #7:
Yes, a separate patch should be written for each of the (up to 6) repositories affected, as shown in the following listing (giving the top-level directory of each embedded repository relative to your comm-central clone). For each file the lowest directory tree containing it is what counts.

comm-central    is ./
mozilla-central is ./mozilla
chatzilla       is ./mozilla/extensions/irc
DOM Inspector   is ./mozilla/extensions/inspector
LDAP SDK        is ./ldap/sdks
Venkman         is ./mozilla/extensions/venkman

And once the patches are ready, a separate commit should be done for each of them, then a push to the appropriate repository. Normally this push is taken care of by some "trusted" developer once you set the checkin-needed flag.

Comment 9

[Philip Chee]

Dale, is your patch ready for review? If you you need to flag it for review and set the requestee flag to Neil@parkway

Comment 10

[daletron]

Hey Philip sorry for the delay, was going to try and find someone to run my patch with the Thunderbird test servers that you were talking about but been a little busy with another project.  I'm going to go ahead and flag it for review and see how it goes.

Comment 11

[[e-mail]]

Comment on attachment 626678 [details] [diff] [review]
a patch that attempts to fix the bug

Worked on my local Linux build and on Thunderbird Try (well, there was some other bustage but I don't think it was caused by the patch.)

Comment 12

[Mozilla RelEng Bot]

Try run for 6cf6df7e2add is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6cf6df7e2add
Results (out of 34 total builds):
    exception: 1
    success: 9
    warnings: 18
    failure: 6
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-6cf6df7e2add

Comment 13

[Philip Chee]

> https://tbpl.mozilla.org/?tree=Try&rev=6cf6df7e2add
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-6cf6df7e2add
These links don't work. Links expired?

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/e495782be1f6

Comment 15

[daletron]

Hey guys, noticed Ryan changed the status/resolution to resolved and fixed.  Just to clarify, does that mean everything is fixed and nothing further needs to be done?

Because according to the Mozilla RelEng Bot post, i see an exception and 6 failures... so I'm assuming otherwise.

Comment 16

[Philip Chee]

>  Comment 15     daletron (New to Bugzilla)      2012-07-12 17:46:05 PDT
> i see an exception and 6 failures... so I'm assuming otherwise.
See below:

>  Comment 11     [e-mail]      2012-07-11 14:20:36 PDT
> (well, there was some other bustage but I don't think it was caused by the patch.)

Thanks Dale!

Comment 17

[daletron]

Thanks Philip and everyone else for your help!