r61176 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r61175‎ | r61176 | r61177 >
Date:18:27, 17 January 2010
Author:overlordq
Status:resolved (Comments)
Tags:
Comment:
First abortive attempt
Modified paths:
  • /branches/overlordq/wikibugs (modified) (history)

Diff [purge]

Index: branches/overlordq/wikibugs
@@ -1,157 +1,114 @@
2 -#!/usr/bin/perl -w
 2+#!/usr/bin/perl
33
4 -# Script to pull bug info from SourceForce and MediaZilla mails and dump them
5 -# to a log to be used for IRC bot.
6 -
7 -# Original version by Brion Vibber, 2004-08-02, 2004-08-10 and 2004-08-15
8 -# Entirely rewritten by Timwi, 2004-09-06
9 -# Some cleanups and fixes by AzaToth, 2006-12-20
10 -
114 use strict;
12 -use utf8;
 5+use warnings;
 6+use Email::MIME;
137
14 -$/ = undef;
15 -my $contents = <STDIN>;
 8+my $input;
169 my $output;
 10+Slurp in E-Mail from STDIN
 11+{
 12+ local( $/ );
 13+ $input = <STDIN>;
 14+}
1715
18 -my $shash = {
19 - 'enhancement' => "\00315enhancement\003",
20 - 'trivial' => 'trivial',
21 - 'minor' => "minor",
22 - 'normal' => "normal",
23 - 'major' => "major",
24 - 'critical' => "\00304CRIT\003",
25 - 'blocker' => "\00304\002BLOCKER\002\003"
 16+##Setup basic settings
 17+my $severityMap = {
 18+ 'enhancement' => "\00315enhancement\003",
 19+ 'trivial' => "trivial",
 20+ 'minor' => "minor",
 21+ 'normal' => "normal",
 22+ 'major' => "major",
 23+ 'critical' => "\00304CRIT\003",
 24+ 'blocker' => "\00304\002BLOCKER\002\003"
2625 };
2726
28 -my $rhash = {
29 - 'WORKSFORME' => "\00314(WFM)\003",
30 - 'INVALID' => "\00314(INVALID)\003",
31 - 'DUPLICATE' => "\00314(DUP)\003",
32 - 'FIXED' => "\00303(FIXED)\003",
33 - 'WONTFIX' => "\00303(WONTFIX)\003",
34 - 'LATER' => "\00306(LATER)\003",
35 - 'REMIND' => "\00306(REMIND)\003"
 27+my $resolutionMap = {
 28+ 'WORKSFORME' => "\00314(WFM)\003",
 29+ 'INVALID' => "\00314(INVALID)\003",
 30+ 'DUPLICATE' => "\00314(DUP)\003",
 31+ 'FIXED' => "\00303(FIXED)\003",
 32+ 'WONTFIX' => "\00303(WONTFIX)\003",
 33+ 'LATER' => "\00306(LATER)\003",
 34+ 'REMIND' => "\00306(REMIND)\003"
3635 };
3736
38 -use Email::MIME;
 37+my $mail = Email::MIME->new( $input );
3938
40 -my $mail = Email::MIME->new( $contents );
41 -
4239 my $from = $mail->header( 'From' );
4340 my $body = $mail->body;
4441 my $subject = $mail->header( 'Subject' );
4542
46 -if ($from =~ /^bugzilla-daemon/)
47 -{
48 - # E-mail is from MediaZilla
 43+if( $from =~ /^bugzilla-daemon/ ) {{
 44+ #E-Mail is from WM Bugzilla
 45+ #Create status flags;
 46+ my ($haschanges,$user);
4947
50 - $/ = "";
51 - # extract and remove the comment section
52 - my ($haschanges, $user);
53 - my $comment = $body =~ s/^--- Comment #\d+ from (?-s:.*?<)?(\S+)\@.*//ms;
 48+ my $comment = $body =~ s/^--- Comment #\d+ from (.*?)?<?(\S+)\@.*//ms;
5449 if($comment) {
55 - $user = $1;
 50+ #Set user to Real name if specified else use e-mail
 51+ $user = $1 ? $1 : $2;
 52+ $user =~ s/\s+$//g;
5653 } else {
57 - $user = 'N/A' # shouldn't be possible'
 54+ $body =~ m/(.*)\s+<(\S+\@\S+)>\s+changed/;
 55+ $user = $1 ? $1 : 'N/A';
5856 }
59 -
 57+
6058 my @changed_fields = split /\s+/, $mail->header( 'X-Bugzilla-Changed-Fields' );
6159
 60+ #Skip dependency notifications
 61+ last if $body =~ /^Bug \d+ depends on bug \d+, which changed state/m;
6262
63 - # Check if this is a dependency e-mail. If so, ignore it.
64 - # We have removed the comment section to prevent people from using
65 - # this by adding the right text to a comment.
66 - if ($body !~ /^Bug \d+ depends on bug \d+, which changed state/m)
67 - {
 63+ my($bug, $summary, $status);
6864
69 - my ($bug, $summary, $st);
70 - if ($subject =~ /\[Bug (\d+)\]\s+New:\s+(.*)/s) {
71 - ($bug, $summary, $st) = ($1, $2, "\00303(NEW)\003");
72 - } elsif ($subject =~ /\[Bug (\d+)\]\s(.*)/s) {
73 - ($bug, $summary, $st) = ($1, $2, "\00303(mod)\003");
74 - }
75 -
76 - ## Set the URL to the URL found in the message body if available, else construct our own URL
77 - my $url =
78 - $body =~ /^http.*show_bug\.cgi.*$/m
79 - ? $&
80 - : "http://bugzilla.wikimedia.org/show_bug.cgi?id=$bug";
 65+ ($bug, $summary) = $subject =~ /\[Bug (\d+)\]\s(.*)/s;
 66+
 67+ $status = $mail->header( 'X-Bugzilla-Status' );
8168
82 - $summary =~ s/\s+/ /g;
 69+ ##Get the url from the message
 70+ my ($bugId) = $mail->header( 'In-Reply-To' ) =~ m/bug-(\d+)/;
 71+ my $url = 'https://bugzilla.wikimedia.org/show_bug.cgi?id=' . $bugId;
8372
84 - # We are going to append stuff to the beginning of $output later.
85 - # This stuff is going to contain $st. But we want a chance of changing it first.
86 - $output = "";
 73+ $summary =~ s/\s+/ /g;
8774
88 - if ($st eq "\00303(NEW)\003")
89 - {
90 - my $product = $mail->header( 'X-Bugzilla-Product' );
91 - my $component = $mail->header( 'X-Bugzilla-Component' );
92 - my $severity = $mail->header( 'X-Bugzilla-Severity' );
93 - ## Doesn't seem to be sent as a header.
94 - my $reporter = $1 if $body =~ /ReportedBy: (.*)\@.*$/m;
 75+ if( $subject =~ /\[Bug (\d+)\]\s+New:(.*)/s ) {
 76+ my $product = $mail->header( 'X-Bugzilla-Product' );
 77+ my $component = $mail->header( 'X-Bugzilla-Component' );
 78+ my $severity = $mail->header( 'X-Bugzilla-Severity' );
 79+ my $priority = $mail->header( 'X-Bugzilla-Priority' );
 80+ my $reporter = $mail->header( 'X-Bugzilla-Who' );
 81+# my $reporter = $1 if $body =~ /ReportedBy: (.*)\@.*$/m;
9582
96 - $output .= "$severity; \002$product\002\: $component; (\002$reporter\002)\n";
97 - }
98 - else
99 - {
100 - ($haschanges, $user) = (1, $1) if $contents =~ /^(?:.*<)?(\S+)\@\S+>? changed:$/m;
101 -
102 - if ($haschanges) {
103 - my @outputs;
104 - my $status = $mail->header( 'X-Bugzilla-Status' );
105 - if ($status eq 'NEW') {
106 - $st = "\00303(mod)\003";
107 - } elsif ($status eq 'REOPENED' && grep {$_ eq 'Status'} @changed_fields) {
108 - $st = "\00304(REOPENED)\003";
109 - } elsif ( grep {$_ eq 'Status'} @changed_fields ) {
110 - $st = "\00303($status)\003";
111 - } else {
112 - $st = "\00303(mod)\003";
113 - }
114 -
115 - if ($st eq "\00303(RESOLVED)\003" && $body =~ /Resolution\|\s+\|(\w+)/m) {
116 - $st = $rhash->{$1};
117 - }
118 - if ($body =~ /Severity\|(\w+)\s+\|(\w+)/m) {
119 - push @outputs, "$shash->{$1}\->$shash->{$2}";
120 - }
121 - if ($body =~ /Keywords\|.*$/s) {
122 - my @lines = split (/\n/, $&);
123 - my $added = '';
124 - my $removed = '';
125 - foreach my $a ( @lines )
126 - {
127 - last unless $a =~ /^(Keywords|\s+)\|(.*?)\s*\|(.*?)\s*$/;
128 - $removed .= $2;
129 - $added .= $3;
130 - }
131 - push @outputs, join ' ', (
132 - ($removed =~ /\S/ ? join (' ', map { "-$_" } split (/\s*,\s*/, $removed)) : ''),
133 - ($added =~ /\S/ ? join (' ', map { "+$_" } split (/\s*,\s*/, $added )) : '')
134 - );
135 - }
136 -
137 - push @outputs, 'summary' if $body =~ /Summary\|.*?\|.*?/;
138 -
139 - push @outputs, 'deps' if $body =~ /OtherBugs\w+\|.*?\|.*?$/m;
140 -
141 - push @outputs, "+comment" if $comment;
142 -
143 - $output .= " " . join ('; ', @outputs) if @outputs;
144 -
 83+ $output = "\00303(NEW)\003 $summary - \00310$url\003 " . $severityMap->{$severity} .
 84+ "; $priority; \002$product\002\: $component; (\002$reporter\002)\n";
 85+ } else {
 86+ if( $mail->header( 'X-Bugzilla-Type' ) eq 'newchanged' ) {
 87+ my @outputs;
 88+ my $status = "\00303(mod)\003";
 89+ if($body =~ /Severity\|(\w+)\s+\|(\w+)/m) {
 90+ push(@outputs, $severityMap->{$1} . '->' . $severityMap->{$2});
14591 }
146 - $output .= " (\002\00310$user\003\002)\n";
 92+ if($body =~ /Resolution\|(\w+)?\s+\|(\w+)/m ) {
 93+ $status = $resolutionMap->{$2};
 94+ }
 95+ if($body =~ /Priority\|(\w+)\s+\|(\w+)/m ) {
 96+ push(@outputs, lc($1) . '->' . lc($2) );
 97+ }
 98+ if($body =~ /Keywords\|.*$/s) {
 99+ }
 100+ push @outputs, 'summary' if grep{$_ eq 'Summary'} @changed_fields;
 101+ push @outputs, 'deps' if grep{$_ eq 'OtherBugs'} @changed_fields;
 102+ push @outputs, '+comment' if $comment;
 103+
 104+ $output = "$status $summary - \00310$url\003 ";
 105+ $output .= join('; ', @outputs) if @outputs;
147106 }
148 - $output = "$st $summary - \00310$url\003 " . $output;
 107+ $output .= " (\002\00310$user\003\002)\n";
149108 }
150 -}
 109+}}
151110
152 -if ($output)
153 -{
154 - open (OUT, ">>/var/wikibugs/wikibugs.log");
155 -# print $output;
 111+if ($output) {
 112+ open( OUT, ">>/var/wikibugs/wikibugs.log");
156113 print OUT $output;
157114 close OUT;
158115 }

Comments

#Comment by Simetrical (talk | contribs)   23:28, 17 January 2010

You really shouldn't remove copyright notices from files.

#Comment by OverlordQ (talk | contribs)   23:50, 17 January 2010

That would imply I started with the original code, which I didn't, I'll add it back when I'm done. In my hasten to strip out all the commented code, I rm'ed too many hashes so this wont even work in the first place.

#Comment by Simetrical (talk | contribs)   23:58, 17 January 2010

What you mean you didn't start with the original code? Are you saying things like

-			my $product = $mail->header( 'X-Bugzilla-Product' );
-			my $component = $mail->header( 'X-Bugzilla-Component' );
-			my $severity = $mail->header( 'X-Bugzilla-Severity' );
-			## Doesn't seem to be sent as a header.
-			my $reporter = $1 if $body =~ /ReportedBy: (.*)\@.*$/m;
+		my $product 	= $mail->header( 'X-Bugzilla-Product' );
+		my $component 	= $mail->header( 'X-Bugzilla-Component' );
+		my $severity	= $mail->header( 'X-Bugzilla-Severity' );
+		my $priority	= $mail->header( 'X-Bugzilla-Priority' );
+		my $reporter	= $mail->header( 'X-Bugzilla-Who' );
+#		my $reporter	= $1 if $body =~ /ReportedBy: (.*)\@.*$/m;

just happen to be byte-for-byte identical, with identical ordering and identical choice of variable names, by sheer coincidence? And in fact you not only wrote the exact same "my $reporter" line, but then independently decided it was useless and commented it out?

#Comment by OverlordQ (talk | contribs)   00:03, 18 January 2010
  1. Open old code.
  2. Open new code.
  3. Copy from old code
  4. Paste into new code.
  5. Getting uppity at somebody for not copying extraneous code comments when debugging is being just a bit too pedantic.

Like I said, I'll add it when it's done.

#Comment by OverlordQ (talk | contribs)   00:05, 18 January 2010

And the reason it's there but commented out is because it was working fine for most cases, but it didn't catch edge cases which I came across so I commented it out which is what I tend to do when developing when I dont know for sure if what I come up with is better then the original so I have a reference point to easily switch back by adding or removing a comment character.

Status & tagging log