The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in Thunderbird 16.0

Status

MailNews Core
Backend
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Philip Chee, Assigned: daletron)

Tracking

Trunk
Thunderbird 16.0
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment)

Comment hidden (empty)
(Reporter)

Comment 1

6 years ago
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.)
(Reporter)

Updated

5 years ago
Whiteboard: [good first bug] → [good first bug][mentor=Neil][lang=c++]
(Assignee)

Comment 2

5 years ago
Hi, me and a couple of students would like to take on this bug.  Can we be assigned to this?
(Reporter)

Comment 3

5 years ago
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
(Assignee)

Comment 4

5 years ago
Created attachment 626678 [details] [diff] [review]
a patch that attempts to fix the bug
(Assignee)

Comment 5

5 years ago
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.
(Reporter)

Comment 6

5 years ago
> "hg diff" never recognizes it.
Try "hg qrefresh"
(Assignee)

Comment 7

5 years ago
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.
(Reporter)

Updated

5 years ago
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
(Reporter)

Comment 9

5 years ago
Dale, is your patch ready for review? If you you need to flag it for review and set the requestee flag to Neil@parkway
(Assignee)

Comment 10

5 years ago
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.
(Assignee)

Updated

5 years ago
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+

Comment 12

5 years ago
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
(Reporter)

Comment 13

5 years ago
> 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
Last Resolved: 5 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
(Assignee)

Comment 15

5 years ago
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.
(Reporter)

Comment 16

5 years ago
>  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!
(Assignee)

Comment 17

5 years ago
Thanks Philip and everyone else for your help!
You need to log in before you can comment on or make changes to this bug.