Closed
Bug 174988
Opened 22 years ago
Closed 16 years ago
post_bug.cgi and process_bug.cgi need to be combined
Categories
(Bugzilla :: Creating/Changing Bugs, enhancement)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: jeff.hedlund, Assigned: romaia)
References
Details
Attachments
(1 file, 2 obsolete files)
33.07 KB,
patch
|
mkanat
:
review-
|
Details | Diff | Splinter Review |
post_bug and process_bug have a lot of common code and need to be combined.
Comment 1•22 years ago
|
||
This needs to be done via Bug.pm - its the sensible way, + the way of the future!
Depends on: 171493
Updated•19 years ago
|
Severity: normal → enhancement
OS: other → All
Hardware: Other → All
Assignee | ||
Comment 2•19 years ago
|
||
Assignee | ||
Updated•19 years ago
|
Attachment #188450 -
Flags: review?
Updated•19 years ago
|
Attachment #188450 -
Flags: review?(LpSolit)
Comment 3•19 years ago
|
||
Essentially this patch moves the Bug-specific code from post_bug to Bug.pm. It's pretty much a cut-n-paste job with some added methods to do updating of the Bug object. The cool part about this is that process_bug can use these methods and become quite a bit simpler. This is one of the large cleanups we need to do for Bugzilla in order to survive. Fixing a dependency -- it was really blocks. Fred, do you think we should split this part into a separate bug?
Comment 4•19 years ago
|
||
kiko, please leave this dependency alone!! bug 242318 is a bug we want to fix for 2.20. Yours is targetted 2.22. And yes, this patch should be splitted into smaller pieces... as soon as the actual blockers and the security patches have landed (this patch will bitrot very soon). The right way to go is to clean up both process_bug.cgi and post_bug.cgi at the same time, meaning that the code doing the same job must be moved at the same time. What is done here is not efficient, as a routine which fits perfectly for post_bug.cgi has a high probability to need some modifications to suit for process_bug.cgi.
Comment 5•19 years ago
|
||
A few points: a) The reason I changed the depenency was that it becomes very easy to fix the bug 242318 once this bug is fixed; I'm sorry I didn't notice the milestone differed. b) This dependency is pretty much useless as it stands, then -- there is no patch on the dependency, and there is one here. The fact that the change there will bitrot this patch doesn't seem to important enough to justify a dependency -- any other patch which touches post_bug will do this. c) I completely refuse to accept that both process_bug and post_bug need to be done at the same time. This is plain blocking good work done -- if you say "this can't be done incrementally" then the odds are it won't be done at all. There is nothing that stops us from doing post_bug and then adapting the methods when process_bug is done -- in fact, that is the way it /should/ be done. d) There is nothing to split this patch out into if the review process isn't as slow as molasses -- it simply cut-n-pastes code out of post_bug.cgi, and the swifter, and the least that changes in that process, the better it is. The only splitting that I suggested was splitting this bug off into a "Convert post_bug.cgi to use Bug.pm"-themed bug, which I'll ask Ronaldo to do now. I can do the review myself if noone else makes himself available in able time.
Comment 6•19 years ago
|
||
Comment on attachment 188450 [details] [diff] [review] clean up on post_bug.cgi >+for my $field ($cgi->param()) { >+ if ( scalar(@{%form_data->{$field}} = $cgi->param($field)) == 1) { >+ %form_data->{$field} = $cgi->param($field); > } > } Great! We did deFORMication (removing %::FORM stuff) and you bring it back here. >+my $bug = Bugzilla::Bug::CreateBug(\%form_data, $comment); Why has $comment a special status? >+ &::SendSQL("SELECT user_id FROM user_group_map Deprecated routines. Please use DBI stuff. Moreover, the patch doesn't apply cleanly anymore, see the dependent bugs.
Attachment #188450 -
Flags: review?(LpSolit)
Attachment #188450 -
Flags: review?
Attachment #188450 -
Flags: review-
Assignee | ||
Comment 7•19 years ago
|
||
Attachment #188450 -
Attachment is obsolete: true
Attachment #191243 -
Flags: review?
Assignee | ||
Comment 8•19 years ago
|
||
fixing bitrot
Attachment #191243 -
Attachment is obsolete: true
Attachment #191524 -
Flags: review?(LpSolit)
Comment 9•19 years ago
|
||
Comment on attachment 191524 [details] [diff] [review] v4-romaia - minor changes Nit: -if (defined $cgi->param('product')) { - if (defined $cgi->param('version')) { +if (defined $cgi->param('product') && defined $cgi->param('version')) { $cgi->send_cookie(-name => "VERSION-$product", -value => $cgi->param('version'), -expires => "Fri, 01-Jan-2038 00:00:00 GMT"); - } } That needs to have the identation fixed as well.
Assignee | ||
Updated•19 years ago
|
Attachment #191524 -
Flags: review?(mkanat)
Comment 10•19 years ago
|
||
Would it be at all possible to first add those "add_" functions to Bugzilla::Bug in another bug, and move process_bug code to use them, and then do the whole CreateBug thing in a separate bug? This patch is just too big to review over and over, or even once. Also, by the summary of this bug, all this work should be happening in a blocker, not in this bug, anyhow.
Comment 11•19 years ago
|
||
Comment on attachment 191524 [details] [diff] [review] v4-romaia - minor changes Yeah, in fact, this definitely does generally need to be split into smaller patches somehow.
Attachment #191524 -
Flags: review?(mkanat) → review-
Updated•19 years ago
|
Attachment #191243 -
Flags: review?
Updated•19 years ago
|
Attachment #191524 -
Flags: review?(LpSolit)
Updated•19 years ago
|
Version: unspecified → 2.21
Updated•19 years ago
|
Depends on: bz-postbugpm
Comment 12•19 years ago
|
||
The trunk is now frozen to prepare Bugzilla 2.22. Enhancement bugs are retargetted to 2.24.
Target Milestone: Bugzilla 2.22 → Bugzilla 2.24
Updated•18 years ago
|
QA Contact: mattyt-bugzilla → default-qa
Updated•18 years ago
|
Comment 13•18 years ago
|
||
This bug is retargetted to Bugzilla 3.2 for one of the following reasons: - it has no assignee (except the default one) - we don't expect someone to fix it in the next two weeks (i.e. before we freeze the trunk to prepare Bugzilla 3.0 RC1) - it's not a blocker If you are working on this bug and you think you will be able to submit a patch in the next two weeks, retarget this bug to 3.0. If this bug is something you would like to see implemented in 3.0 but you are not a developer or you don't think you will be able to fix this bug yourself in the next two weeks, please *do not* retarget this bug. If you think this bug should absolutely be fixed before we release 3.0, either ask on IRC or use the "blocking3.0 flag".
Target Milestone: Bugzilla 3.0 → Bugzilla 3.2
Updated•18 years ago
|
Target Milestone: Bugzilla 3.2 → ---
Comment 14•16 years ago
|
||
Max, agree to mark this bug as WONTFIX? I see no benefit in merging post_bug.cgi and process_bug.cgi together.
Comment 15•16 years ago
|
||
Yes, agreed.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•