If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Move GetLongDescriptionAsText out of globals.pl

RESOLVED FIXED in Bugzilla 2.22

Status

()

Bugzilla
Email Notifications
--
enhancement
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Frédéric Buclin)

Tracking

(Blocks: 1 bug)

2.19.2
Bugzilla 2.22
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(1 attachment)

(Reporter)

Description

13 years ago
Really, this should be something like get_comments_for_bug_during_period, and
it's only used by BugMail.pm. It does seem like something that should be in
Bug.pm, though. I'm not sure... any comments?
(Reporter)

Updated

13 years ago
Blocks: 87411
(Reporter)

Updated

13 years ago
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.22

Comment 1

13 years ago
FYI, I've moved this into BugMail.pm as part of bug #116863
(Reporter)

Comment 2

13 years ago
OK. One day we need to make that function not suck, but that will probably
happen when we templatize email.
Assignee: mkanat → altlst
Depends on: 116863
Whiteboard: [blocker will fix]
(Assignee)

Comment 3

13 years ago
Move this function separately. Marking this bug as blocking the other one as the
patch in 116863 has been denied review.
Blocks: 116863
No longer depends on: 116863
Whiteboard: [blocker will fix]
(Assignee)

Comment 4

12 years ago
The move to BugMail.pm is trivial. Why isn't it done yet?
(Reporter)

Comment 5

12 years ago
Because nobody's patched it, I'd guess. :-)
Assignee: altlst → email-notifications

Comment 6

12 years ago
(In reply to comment #4)
> The move to BugMail.pm is trivial. Why isn't it done yet?

Yea, I've been tied up getting 2.20 installed at my company (among other things)
and only had time to use the original patch in bug #116863.  When things die
down, I can look into breaking it up. Or somebody else could do it.

My only request is that there should still be a "get_all_comments" routine,
which I believe would be needed for bug #116863
 
(Assignee)

Comment 7

12 years ago
Let's move it in BugMail.pm for now. As mkanat said, it's only used in
BugMail.pm and moving this routine in Bug.pm would create a module dependency
loop between BugMail.pm and Bug.pm.
Assignee: email-notifications → LpSolit
(Assignee)

Comment 8

12 years ago
Created attachment 194173 [details] [diff] [review]
patch, v1

I'm sure mkanat wants to review this one. After all, you are the one who opened
this bug. :-p

Note that GetLongDescriptionAsText() has been renamed as
BugMail::get_comments_by_bug().
Attachment #194173 - Flags: review?(mkanat)
(Reporter)

Comment 9

12 years ago
Comment on attachment 194173 [details] [diff] [review]
patch, v1

r=mkanat by inspection.

Some day this whole sub needs to die (probably as part of the BugMail re-work
in bug 301447).
Attachment #194173 - Flags: review?(mkanat) → review+
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Flags: approval?
Priority: P2 → --
Flags: approval? → approval+
(Assignee)

Comment 10

12 years ago
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.336; previous revision: 1.335
done
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.51; previous revision: 1.50
done
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.