Closed Bug 695255 Opened 8 years ago Closed 7 years ago

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

Categories

(MailNews Core :: Backend, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 16.0

People

(Reporter: philip.chee, Assigned: questionth1s)

References

Details

(Whiteboard: [good first bug][mentor=Neil][lang=c++])

Attachments

(1 file)

No description provided.
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.)
Whiteboard: [good first bug] → [good first bug][mentor=Neil][lang=c++]
Hi, me and a couple of students would like to take on this bug.  Can we be assigned to this?
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.
Assignee: nobody → questionth1s
Status: NEW → ASSIGNED
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.
> "hg diff" never recognizes it.
Try "hg qrefresh"
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?
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.
Summary: Replace uses of NS_ARRAY_LENGTH with mozilla::ArrayLength and mozilla::ArrayEnd in Suite → Replace uses of NS_ARRAY_LENGTH with mozilla::ArrayLength and mozilla::ArrayEnd in MailNews
Dale, is your patch ready for review? If you you need to flag it for review and set the requestee flag to Neil@parkway
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.
Attachment #626678 - Flags: review?(neil)
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.)
Attachment #626678 - Flags: review?(neil) → review+
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/neil@parkwaycc.co.uk-6cf6df7e2add
> https://tbpl.mozilla.org/?tree=Try&rev=6cf6df7e2add
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@parkwaycc.co.uk-6cf6df7e2add
These links don't work. Links expired?
Component: General → Backend
Keywords: checkin-needed
Product: SeaMonkey → MailNews Core
https://hg.mozilla.org/comm-central/rev/e495782be1f6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
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 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     neil@parkwaycc.co.uk      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!
Thanks Philip and everyone else for your help!
You need to log in before you can comment on or make changes to this bug.