Move dependency DB updating from process_bug into Bugzilla::Bug

RESOLVED FIXED in Bugzilla 3.2

Status

()

Bugzilla
Creating/Changing Bugs
--
enhancement
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

Bugzilla 3.2
Dependency tree / graph
Bug Flags:
approval +

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

11 years ago
After keywords and CCs, the next thing that I can move into the DB is dependencies.
(Assignee)

Comment 1

11 years ago
Created attachment 269301 [details] [diff] [review]
v1

Okay, here we go! This applies against the keywords patch. I've tested it and it works fine, and this is a LOT cleaner than the old code.

Believe it or not, this code really does to everything that the old code did. It also never checks dependencies for changes if they don't need to be notified (which is an improvement over the old code that would always check dependencies, in certain cases, even if they didn't need to be notified).
Attachment #269301 - Flags: review?(LpSolit)
(Assignee)

Updated

11 years ago
Depends on: 384664
(Assignee)

Comment 2

11 years ago
Created attachment 269768 [details] [diff] [review]
v1.1

Fixed some minor bitrot.
Attachment #269301 - Attachment is obsolete: true
Attachment #269768 - Flags: review?(LpSolit)
Attachment #269301 - Flags: review?(LpSolit)

Comment 3

11 years ago
Comment on attachment 269768 [details] [diff] [review]
v1.1

>diff -u Bugzilla/Bug.pm Bugzilla/Bug.pm

>+sub update_dependencies {

Nit: please cleanup all these lines containing whitespaces only.


>+sub set_dependencies {

>+    trick_taint($_) foreach (@$dependson, @$blocked);

AFAICS, these values are already detained by ValidateBugID(), so calling trick_taint() here is useless (and it would even be detaint_natural() as ValidateBugID converts aliases to bug IDs). I suggest you remove this line to make clear where the validation occurs.

Your patch seems to work correctly. r=LpSolit
Attachment #269768 - Flags: review?(LpSolit) → review+

Updated

11 years ago
Flags: approval+
(Assignee)

Comment 4

11 years ago
Okay, all done! I wasn't sure what you meant about the whitespace lines. There were some line breaks in that subroutine, but they were there on purpose, for the most part.

Instead of removing the trick_taint, I added a comment explaining why it's there, and made it a detaint_natural.

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.372; previous revision: 1.371
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.186; previous revision: 1.185
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 5

11 years ago
Created attachment 271360 [details] [diff] [review]
fixing bustage

The 'list_comparison_error' error tag is no longer used, which makes |runtests.pl 12| to fail. I'm applying this patch to fix bustage.

Comment 6

11 years ago
Tinderbox should be happy now.

Checking in template/en/default/global/code-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/code-error.html.tmpl,v  <--  code-error.html.tmpl
new revision: 1.98; previous revision: 1.97
done

Updated

11 years ago
Blocks: 387705
You need to log in before you can comment on or make changes to this bug.