472217 - Need a Bugzilla::Comment object

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

The more I work with comments, the more apparent it becomes that they should be objects. They can be Bugzilla::Object-based even, since they have an id.

Comment 1

[arbingersys]

(In reply to comment #0)
I'd like to try and take a crack at this, if I may. Any guidance you could give? 

I've been poking around the source, and it seems like a fairly straightforward task. We'd basically be making a Bugzilla::Object descendant out of the longdescs table. Is there a current Bugzilla::Object descendant that would be good to copy and modify as a sort of template?

Also, it seems like Bugzilla::Bug::GetComments() would return the Bugzilla::Bug::Comments (Bugzilla::Comments?) object, so that templates would continue to be able to do

[% PROCESS bug/comments.html.tmpl                                          
   comments = bug.longdescs
   mode = user.id ? "edit" : "show"                                        
%]

My observations are fairly perfunctory at this point. Anything else I might need to be aware of? Thanks, James

Comment 2

[Max Kanat-Alexander]

(In reply to comment #1)
> Is there a current Bugzilla::Object descendant that would be good to copy and modify as a sort of template?

  Bugzilla::Keyword is usually the simplest object to look at.

> Also, it seems like Bugzilla::Bug::GetComments() would return the
> Bugzilla::Bug::Comments (Bugzilla::Comments?) object, so that templates would
> continue to be able to do

  Yeah, that would be ideal, but when you first create the object, you should use it in the least invasive way possible and it should do as little as possible. Then we can build on that in later bugs.

Comment 3

[Emmanuel Seyman]

(In reply to comment #1)
> 
> Also, it seems like Bugzilla::Bug::GetComments() would return the
> Bugzilla::Bug::Comments (Bugzilla::Comments?) object

An array of Bugzilla::Comment objects sounds like the most appropriate choice here.

Comment 4

[Max Kanat-Alexander]

(In reply to comment #3)
> An array of Bugzilla::Comment objects sounds like the most appropriate choice
> here.

  Of course.

Comment 5

[arbingersys]

Attachment: Bugzilla::Comment class

Here's the package. The only things not essentially boiler plate are a couple of accessors so the templates would be happy. I also added a validator for the 'thetext' field, but I'm not sure whether it was necessary or not.

Comment 6

[arbingersys]

Attachment: patch to Bugzilla::Bug::GetComments

GetComments now queries for a list of comment_ids for a bug, and does Comment->new_from_list().

Comment 7

[Max Kanat-Alexander]

Comment on attachment 366893 [details]
Bugzilla::Comment class

># The Original Code is the Bugzilla Bug Tracking System.

  You are missing the Initial Developer block.

>use constant DB_COLUMNS => qw(
>    longdescs.comment_id

  You don't need the table name in front of the column names. (I know that we do this elsewhere, but it's actually unnecessary.)

>use constant DB_TABLE => 'longdescs';
>
>use constant ID_FIELD => 'comment_id';
>
>use constant LIST_ORDER => 'bug_when';

  Nit: Don't need an extra newline between these.

>use constant VALIDATORS => { 
>    thetext => \&_check_thetext
>};

  I'm fairly sure that everything needs a validator, but don't implement create() yet. Wait for another bug. We never implement anything unless it's actually being used.

>use constant UPDATE_COLUMNS => qw(

  Same comment here, don't implement update() until you're using it somewhere.

>sub bug_id   { return $_[0]->{'bug_id'}; }
>sub who      { return $_[0]->{'who'}; }
>sub userid   { return who(); }

  userid is a bad name for a field, and all names should be canonical. If the templates are using a bad name, then the templates should just be changed.

>sub bug_when { return $_[0]->{'bug_when'}; }

  bug_when is probably not a good standard name for a field. Perhaps creation_ts would be a better name for the accessor to be consistent with the rest of Bugzilla?

>sub time     { return bug_when(); }

  Same comment here. (Also, even if you were to do this, you would need to call it in $_[0].)

>###############################
>####       Mutators       #####
>###############################

  Let's not worry about these for now.

>=head1 SYNOPSIS
>
> use Bugzilla::Comment;
>
> my $comment = Bugzilla::Comment->new($comment_id);
>
> my $comments = Bugzilla::Comment->new_from_list($comment_ids);

  Nit: Remove the extra newline between the $comment and $comments line.

Comment 8

[Max Kanat-Alexander]

Comment on attachment 366899 [details] [diff] [review]
patch to Bugzilla::Bug::GetComments

>Index: Bugzilla/Bug.pm

>+    for ( @{Bugzilla::Comment->new_from_list($comment_ids)} ) {
> 
>-        push (@comments, \%comment);
>+        $_->{'author'} = new Bugzilla::User($_->{'userid'});

  That should be an accessor of the Comment object, not created here.

>+        
>+        $_->{'body'} = $_->{'thetext'};
>+        delete($_->{'thetext'});
>+        $_->{'body'} = format_comment($_) unless $raw;

  And perhaps there should be a body and raw_body accessor instead of doing this.

Comment 9

[Max Kanat-Alexander]

  By the way, you can add a new file to a cvs patch by using the "cvsdo" command, like "cvsdo add Bugzilla/Comment.pm". Then a "cvs diff -Nu" will pick it up.

Comment 10

[arbingersys]

(In reply to comment #8)

> >+        $_->{'body'} = $_->{'thetext'};
> >+        delete($_->{'thetext'});
> >+        $_->{'body'} = format_comment($_) unless $raw;
> 
>   And perhaps there should be a body and raw_body accessor instead of doing
> this.

I've gotten most of what you suggest above done (I think). However, I realized that the 'body' returned by Bug->longdescs() is retrieved as a hash element rather than a sub elsewhere in the code, i.e.

post_bug.cgi
209:        if ($bug->longdescs->[0]->{'body'} !~ /^\s+$/) {
210:            $new_comment .= "\n" . $bug->longdescs->[0]->{'body'};

Bugzilla/Bug.pm
2949:    $current_comment_obj[0]->{'body'} = $new_comment;
3048:# $comment{'body'} itself, see BugMail::prepare_comments().

So should I just go through the entire source and change any reference to ...->{'body'} to ...->{'thetext'}, or to the accessor, i.e. ...->body, since 'thetext' is the actual name of the field? Or is it better to assign thetext to 'body' inside the Comment object on instantiation?

Thanks, James

Comment 11

[Max Kanat-Alexander]

All objects should be accessed only through accessors. Their hash elements should never be accessed outside of their own code.

Comment 12

[arbingersys]

Attachment: patch3

Well, I finally got some time to work on this. I think this patch should cover everything discussed. In my testing comments still seem to behave how they did previously. James

Comment 13

[Max Kanat-Alexander]

Comment on attachment 372688 [details] [diff] [review]
patch3

>Index: Bugzilla/Bug.pm
>+    my $query = 'SELECT longdescs.comment_id, longdescs.bug_when 

  You don't need to select bug_when--you're only using comment_id.

>+    my $comment_ids = $dbh->selectcol_arrayref($query, undef, @args);
>+    for ( @{Bugzilla::Comment->new_from_list($comment_ids)} ) {
>+        $_->set_body(format_comment($_)) unless $raw;

  No, instead there should be both a body() and a body_raw() accessor in the Bugzilla::Comment object. body() should return a formatted body, and body_raw() should return an unformatted body.

> sub format_comment {

  And this should be moved to be an appropriate private subroutine within Bugzilla::Comment, probably the code for body().

>Index: Bugzilla/Comment.pm
>+use constant REQUIRED_CREATE_FIELDS => qw(
>+    bug_id
>+    who
>+    bug_when
>+    thetext
>+);
>+
>+use constant VALIDATORS => { 
>+    thetext => \&_check_thetext
>+};

  Don't set those up unless you're going to implement or use create(), which we haven't yet.

>+###############################
>+#####      Mutators       #####
>+###############################
>+
>+sub set_bug_id {
>+    my ($self, $bug_id) = @_;
>+    $self->set('bug_id', $bug_id);
>+}

  Oh, that definitely should not exist. A comment's bug_id is not mutable.

  At this point, actually, there should be no mutators, because we aren't using or implementing update().

>Index: Bugzilla/WebService/Bug.pm
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Bug.pm,v
>retrieving revision 1.34
>diff -p -u -r1.34 Bug.pm
>--- Bugzilla/WebService/Bug.pm	31 Mar 2009 06:37:57 -0000	1.34
>+++ Bugzilla/WebService/Bug.pm	14 Apr 2009 20:45:40 -0000
>@@ -94,8 +94,8 @@ sub comments {
>             $bug->id, 'oldest_to_newest', $params->{new_since});
>         my @result;
>         foreach my $comment (@$comments) {
>-            next if $comment->{isprivate} && !$user->is_insider;
>-            $comment->{bug_id} = $bug->id;
>+            next if $comment->isprivate && !$user->is_insider;
>+            $comment->set_bug_id($bug->id);

  Since you have a comment object, you don't even need to do this. There's a working bug_id accessor. Also, you should change _translate_comment to use accessors instead of direct hash access.

Comment 14

[Max Kanat-Alexander]

Comment on attachment 372688 [details] [diff] [review]
patch3

Oh, also, don't expose "who" as an accessor. Just expose "author".

Comment 15

[Max Kanat-Alexander]

Comment on attachment 372688 [details] [diff] [review]
patch3

Oh, and there's probably no need to expose extra_data (or maybe even type) as external accessors. Also, don't call thetext two different things, just expose a body() accessor and clean up any place referring to it as thetext.

Comment 16

[arbingersys]

Attachment: patch4

> You don't need to select bug_when--you're only using comment_id.

Actually, I left that in so Pg wouldn't break, since bug_when is used in the ORDER BY clause --

https://bugzilla.mozilla.org/show_bug.cgi?id=440259#c14

It probably shouldn't matter, since selectcol_arrayref as it's used only returns the first column anyway.

The $raw param has been removed from GetComments(), meaning you'll need to use the accessor $comment->body_raw in templates or wherever. I didn't see anywhere that it was currently being passed in, so I assume this won't break anything.

I'm not entirely sure if I got the WebService::Bug::comments() changes right, since I couldn't figure out how to test. In ack'ing the source, I couldn't find any calls to the routine, either.

Comment 17

[Max Kanat-Alexander]

(In reply to comment #16)
> Actually, I left that in so Pg wouldn't break, since bug_when is used in the
> ORDER BY clause --
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=440259#c14

  No, you don't need that. The restriction is that when you SELECT DISTINCT, you must have the ORDER BY column in your SELECT. It's not for a normal SELECT.

> I'm not entirely sure if I got the WebService::Bug::comments() changes right,
> since I couldn't figure out how to test. In ack'ing the source, I couldn't find
> any calls to the routine, either.

  It has API docs, you should make sure that it still conforms to them. One way is to use the XMLRPCsh.pl script that comes with SOAP::Lite.

Comment 18

[Frédéric Buclin]

Your patch is going to conflict with bug 457657, which will probably land before yours. You should probably wait for wurblzap's checkin.

Comment 19

[arbingersys]

(In reply to comment #17)
> (In reply to comment #16)

>   No, you don't need that. The restriction is that when you SELECT DISTINCT,
> you must have the ORDER BY column in your SELECT. It's not for a normal SELECT.

Okay, got it.

Comment 20

[Max Kanat-Alexander]

Comment on attachment 372964 [details] [diff] [review]
patch4

>--- Bugzilla/Bug.pm	6 Apr 2009 20:02:51 -0000	1.277
>+++ Bugzilla/Bug.pm	15 Apr 2009 20:48:06 -0000
>
>+    for ( @{Bugzilla::Comment->new_from_list($comment_ids)} ) {
>+        push(@comments, $_);
>     }

  That is unnecessary. You could just do:

  my $comments = Bugzilla::Comment->new_from_list($comment_ids);

  And then dereference the array (@$comments) as necessary.

>Index: Bugzilla/Comment.pm
> [snip]
>+use constant REQUIRED_CREATE_FIELDS => qw(

  We don't need this yet.

>+sub isprivate   { return $_[0]->{'isprivate'}; }

  For consistency, we should probably call that is_private.

>+sub body {
>+    my $self = shift;
>+    my $body;
>+
>+    if ($self->{'type'} == CMT_DUPE_OF) {
>+        $body = $self->{'thetext'} . "\n\n" .

  Use $self->body_raw instead of {thetext}.

>+###############################
>+#####      Mutators       #####
>+###############################
>+
>+# none yet
>+

  Just leave the sections out entirely if they don't have functions yet.

>+=head1 SUBROUTINES
>+
>+None.

  Same here.

  You can document the accessors, by the way. I'd recommend it.

>Index: Bugzilla/WebService/Bug.pm
>@@ -105,11 +105,10 @@ sub comments {
>+            'SELECT comment_id FROM longdescs WHERE ' . 
>+            $dbh->sql_in('comment_id', \@sql_ids), {Slice=>{}});

  Instead of this, you can just call Bugzilla::Comment->new_from_list.

>+        for ( @{Bugzilla::Comment->new_from_list($comment_data)} ) {
>+            if ($_->isprivate && !$user->is_insider) {
>+                ThrowUserError('comment_is_private', { id => $_->id });
>             }

  #1, you should never be doing multi-line foreach loops without a "my" var (and you should be calling them "foreach", not "for").

  #2, you should be creating an arrayref like "my $comments = " and then using it.

Comment 21

[Max Kanat-Alexander]

(In reply to comment #18)
> Your patch is going to conflict with bug 457657, which will probably land
> before yours. You should probably wait for wurblzap's checkin.

  Actually, given the level of activity on both bugs, I suspect this one will land first.

Comment 22

[arbingersys]

Attachment: patch5

Been a bit since I visited this, but I think I got all the changes.

As for testing, runtests.pl was successful, comments work for me on my dev install, and I tested for comments against WebService/Bug.pm with bz_webservice_demo.pl

James

Comment 23

[Max Kanat-Alexander]

Comment on attachment 377484 [details] [diff] [review]
patch5

>Index: Bugzilla/Bug.pm
>+    for ( @{Bugzilla::Comment->new_from_list($comment_ids)} ) {
>+        push(@comments, $_);
>     }

  You don't need to do that. All you need is:

  my $comments = Bugzilla::Comment->new_from_list($comment_ids);

>Index: Bugzilla/BugMail.pm
>     if (Bugzilla->params->{'insidergroup'}) {
>-        $anyprivate = 1 if scalar(grep {$_->{'isprivate'} > 0} @$comments);
>+        $anyprivate = 1 if scalar(grep {$_->is_private > 0} @$comments);

  FWIW, you don't need the "> 0" there.

>Index: Bugzilla/Comment.pm
>+# The Initial Developer of the Original Code is Netscape Communications
>+# Corporation. Portions created by Netscape are
>+# Copyright (C) 1998 Netscape Communications Corporation. All
>+# Rights Reserved.

  I'd say enough of this is yours that you can just say it's yours.

>+sub bug_id      { return $_[0]->{'bug_id'}; }

  Is there ever a time that you need bug_id and not just "bug"? I'd rather have a "bug" accessor that returns a Bugzilla::Bug object (and when Bugzilla::Bug gets comments, it can just pre-populate this field with itself).

[snip]
>+sub body_raw    { return $_[0]->{'thetext'}; }
>+sub work_time   { return $_[0]->{'work_time'}; }
>+sub is_private   { return $_[0]->{'isprivate'}; }
>+sub already_wrapped { return $_[0]->{'already_wrapped'}; }

  By the way, I'd prefer it if these simple accessors were in alphabetical order.

>+=item C<body_raw>
>+
>+raw bytes of the comment

  "The body without any special additional text."

>+=item C<already_wrapped>
>+
>+whether comment formatting has occurred

  "If this comment is stored in the database word-wrapped, this will be C<1>. C<0> otherwise."

>+=item C<author>
>+
>+user who created the comment

  "L<Bugzilla::User> who created the comment"

>+=item C<body>
>+
>+formatted body of the comment

  Body of the comment, including any special text (such as "this bug was marked as a duplicate of...")

>Index: Bugzilla/WebService/Bug.pm
> [snip]
>     if (scalar @$comment_ids) {
>         my @ids = map { trim($_) } @$comment_ids;
>         my @sql_ids = map { $dbh->quote($_) } @ids;

  You don't need to quote the ids anymore if you're just passing them to new_from_list.


  Otherwise it looks pretty good.

Comment 24

[arbingersys]

Attachment: patch6

I added the bug() method, but perhaps I shouldn't have, since nothing is requiring it at the moment.

As to removing the bug_id accessor, it appears to be necessary in WebServices/Bug.pm for the _translate_comment() function.

Comment 25

[Max Kanat-Alexander]

(In reply to comment #24)
> I added the bug() method, but perhaps I shouldn't have, since nothing is
> requiring it at the moment.

  Ah, yeah, if nothing uses it, there's no need for it.

> As to removing the bug_id accessor, it appears to be necessary in
> WebServices/Bug.pm for the _translate_comment() function.

  Yeah, that's fine.

Comment 26

[arbingersys]

Attachment: patch6a

Comment 27

[arbingersys]

Attachment: patch6b

Gah. A little fast with the Enter button. Sorry.

I was returning the wrong hash element in bug() in patch6. This one should be right.

Comment 28

[arbingersys]

Attachment: patch7

Okay, last one (tonight anyway). I removed bug(). James

Comment 29

[Max Kanat-Alexander]

Comment on attachment 377547 [details] [diff] [review]
patch7

>Index: Bugzilla/Bug.pm
>+    my $comment_ids = $dbh->selectcol_arrayref($query, undef, @args);
>+    for ( @$comments ) {
>+        push(@comments, $_);
>     }

  Look, I understand Perl is confusing, but you DON'T have to have that for loop AT ALL. You dereference an arrayref like @$comments. You can do it anywhere to treat an arrayref just like an array.

Comment 30

[Max Kanat-Alexander]

Comment on attachment 377547 [details] [diff] [review]
patch7

>+# The Original Code is the Bugzilla Bug Tracking System.
>+#
>+# Contributor(s): James Robson <[e-mail]> 

  You still need an Original Author section. Have you read the Bugzilla:Developers contribution process? It specifically mentions and links to the licensing guidelines.

>+=item C<bug_id>
>+
>+The bug object to which the comment belongs.

  A bug_id is not an object.

>+
>+=item C<creation_ts>
>+
>+The comment creation timestamp.

  It'd be nice if you could put some types on these. Like C<string> for this one.

>+=item C<already_wrapped>
>+
>+whether comment formatting has occurred

  No, use the text I said.


>         my @ids = map { trim($_) } @$comment_ids;
>-        my @sql_ids = map { $dbh->quote($_) } @ids;

  Actually, come to think of it, we probably don't need that trim anymore either...? I'm not sure, though.

Comment 31

[arbingersys]

Attachment: patch8

I think trim($_) might still be needed, since the comment IDs could come from user input, e.g.

$soapresult = $proxy->call('Bug.comments', {comment_ids => [$comment_id]});

Comment 32

[Max Kanat-Alexander]

Comment on attachment 377738 [details] [diff] [review]
patch8

This actually looks good, but we changed things upstream so that format_comment uses a template now, so this patch isn't quite up-to-date anymore. I'm sorry for the delayed review--somehow I didn't notice the review request until just now.

Comment 33

[arbingersys]

(In reply to comment #32)
> (From update of attachment 377738 [details] [diff] [review])
> This actually looks good, but we changed things upstream so that format_comment
> uses a template now, so this patch isn't quite up-to-date anymore.

So what's next? Re-work to include the format_comment changes? It was this patch, wasn't it --

https://bugzilla.mozilla.org/show_bug.cgi?id=457657

Comment 34

[Max Kanat-Alexander]

Yeah, re-work to include any changes that were made since you posted the patch.

Comment 35

[arbingersys]

Attachment: patch9

Moved format_comment with template to Bugzilla::Comment::body().

Comment 36

[Max Kanat-Alexander]

Comment on attachment 388219 [details] [diff] [review]
patch9

Actually, this looks great. If it doesn't currently apply or has some problems, I will fix them on checkin.

Comment 37

[Max Kanat-Alexander]

Attachment: v10 (WIP)

Okay, so actually, this required more than checkin fixes, but I did all the fixes myself, and it looks pretty nice now, except for one thing--calling the format_comment template strips leading whitespace from comments. :-( I have no idea how to work around it (or at least, no idea how to work around it cleanly).

Comment 38

[Max Kanat-Alexander]

Oh also, the way I have it now won't work right for wants_bug_mail, because of how we're doing "Created an attachment" now.

Comment 39

[Max Kanat-Alexander]

Attachment: v11

Okay, this solves all the earlier problems with the patch. (Jason: Since the patch is now my responsibility, I'm taking the Assigned To field, but I will still credit you during checkin.)

Comment 40

[Max Kanat-Alexander]

By the way, s/Jason/James/ in that last comment. :-)

Comment 41

[arbingersys]

(In reply to comment #40)
> By the way, s/Jason/James/ in that last comment. :-)

No problem. With my name, 'Jim' is always the intentional replacement people make, and 'Jason' the accidental one. :)

Comment 42

[Max Kanat-Alexander]

Attachment: v12

Fixed some bitrot.

Comment 43

[Frédéric Buclin]

Comment on attachment 410108 [details] [diff] [review]
v12

>Index: Bugzilla/Bug.pm

> sub GetComments {

>+    my $query = 'SELECT longdescs.comment_id, longdescs.bug_when 
>+                 FROM longdescs WHERE bug_id = ?';

No need to write longdescs.foo as we only query the longdescs table. Also, bug_when is used nowhere as you only care about comment IDs. It should go away from the query.


>         $query .= ' AND longdescs.bug_when > ?';

s/longdescs\.//; and in the remaining SQL query too.


>     $query .= " ORDER BY longdescs.bug_when $sort_order";

This is useless as new_from_list() already orders the list itself. This ORDER BY will have no effect.



>Index: Bugzilla/User.pm

>+    my $comments_concatenated = join("\n", map { $_->body_full(1) } @$comments);

We don't need the full body here. "Created an attachment" is hardcoded in the comment, and $comments_concatenated is only used for this purpose. This would make email sending a bit faster.



>Index: template/en/default/bug/comments.html.tmpl

>     <div class="bz_comment[% " bz_private" IF comment.isprivate %]

Should be comment.is_private. Appears again later in this template.


>+[% full_comment = BLOCK %]
>+  [% FILTER remove('^X') %][% PROCESS bug/format_comment.txt.tmpl %][% END %]
>+[% END %]

Why not using comment.body_full? This would make the template easier to read.



>Index: template/en/default/bug/format_comment.txt.tmpl

> [% IF comment.type == constants.CMT_DUPE_OF %]
>-X[% comment.body %]
>+X[% comment_body %]

This is not the single place to use comment.body. Other places should be fixed too.



>Index: template/en/default/bug/show.xml.tmpl

>+                [%- FILTER remove('^X') %][% PROCESS "bug/format_comment.txt.tmpl" comment = c %][% END ~%]

Why not using comment.body_full?



>Index: template/en/default/email/newchangedmail.txt.tmpl

>+[%+ FILTER remove('^X') %][% PROCESS bug/format_comment.txt.tmpl is_bugmail = 1 %][% END %]

Same here.

Comment 44

[Max Kanat-Alexander]

Attachment: v13

Okay, I did even better than you asked for. I removed GetComments entirely and gave Bugzilla::Bug objects the functionality that we needed from GetComments. I also renamed the accessor from "longdescs" to "comments", both because it's a better name and because it now returns something so different that I don't want customizers or extension authors to think that they can just keep using it safely, normally (and this also helped me find everywhere that it was being used, and fix it).

And we use body_full everywhere now instead of calling format_comment. In fact, body_full is now the ONLY place where we call format_comment, making our "X" hack much less bad.

Comment 45

[Frédéric Buclin]

Comment on attachment 411305 [details] [diff] [review]
v13

>=== modified file 'Bugzilla/BugMail.pm'

>+    warn scalar(@$comments);

debug code



>=== added file 'Bugzilla/Comment.pm'

>+sub author { 
>+    my $self = shift;
>+    return $self->{'author'} if exists $self->{'author'};
>+    $self->{'author'} = new Bugzilla::User($self->{'who'});

Nit: would be cleaner as: $self->{'author'} ||= new Bugzilla::User(...), so that we have a single return in this method.


Seems to work fine (I extracted datetime_from() from bug 355283, so that this one can land first). r=LpSolit

Comment 46

[Frédéric Buclin]

Would be nice to land datetime_from() as part of this bug, so that I don't need to worry about it anymore when reviewing bug 355283.

Comment 47

[Max Kanat-Alexander]

(In reply to comment #46)
> Would be nice to land datetime_from() as part of this bug, so that I don't need
> to worry about it anymore when reviewing bug 355283.

  Sure, I'd be happy to do that.

Comment 48

[Max Kanat-Alexander]

Checking in email_in.pl;
/cvsroot/mozilla/webtools/bugzilla/email_in.pl,v  <--  email_in.pl
new revision: 1.28; previous revision: 1.27
done
Checking in enter_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v  <--  enter_bug.cgi
new revision: 1.172; previous revision: 1.171
done
Checking in post_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/post_bug.cgi,v  <--  post_bug.cgi
new revision: 1.204; previous revision: 1.203
done
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.424; previous revision: 1.423
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.295; previous revision: 1.294
done
Checking in Bugzilla/BugMail.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/BugMail.pm,v  <--  BugMail.pm
new revision: 1.131; previous revision: 1.130
done
RCS file: /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Comment.pm,v
done
Checking in Bugzilla/Comment.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Comment.pm,v  <--  Comment.pm
initial revision: 1.1
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.113; previous revision: 1.112
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.196; previous revision: 1.195
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.95; previous revision: 1.94
done
Checking in Bugzilla/WebService/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Bug.pm,v  <--  Bug.pm
new revision: 1.45; previous revision: 1.44
done
Checking in template/en/default/bug/comments.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/comments.html.tmpl,v  <--  comments.html.tmpl
new revision: 1.44; previous revision: 1.43
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.168; previous revision: 1.167
done
Checking in template/en/default/bug/format_comment.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/format_comment.txt.tmpl,v  <--  format_comment.txt.tmpl
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/bug/show-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show-multiple.html.tmpl,v  <--  show-multiple.html.tmpl
new revision: 1.46; previous revision: 1.45
done
Checking in template/en/default/bug/show.xml.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.xml.tmpl,v  <--  show.xml.tmpl
new revision: 1.34; previous revision: 1.33
done
Checking in template/en/default/email/newchangedmail.txt.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/email/newchangedmail.txt.tmpl,v  <--  newchangedmail.txt.tmpl
new revision: 1.17; previous revision: 1.16
done

Comment 49

[Max Kanat-Alexander]

Attachment: Checked-In Version

Comment 50

[Max Kanat-Alexander]

Oh dang, I forgot to also credit Jason on the checkin.

Comment 51

[Max Kanat-Alexander]

I mean JAMES. Hahaha. Man, that's the second time IN THIS BUG that I did that! :-)

Comment 52

[Bradley Baetz (:bbaetz)]

(For reference the 'leading X' thing is needed because we use |TRIM => 1|)

Comment 53

[Max Kanat-Alexander]

Added to the release notes in bug 547466.