Closed Bug 174988 Opened 22 years ago Closed 17 years ago

post_bug.cgi and process_bug.cgi need to be combined

Categories

(Bugzilla :: Creating/Changing Bugs, enhancement)

2.21
enhancement
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: jeff.hedlund, Assigned: romaia)

References

Details

Attachments

(1 file, 2 obsolete files)

post_bug and process_bug have a lot of common code and need to be combined.
This needs to be done via Bug.pm - its the sensible way, + the way of the future!
Depends on: 171493
Severity: normal → enhancement
OS: other → All
Hardware: Other → All
Depends on: 242318
Attached patch clean up on post_bug.cgi (obsolete) (deleted) — Splinter Review
Attachment #188450 - Flags: review?
Attachment #188450 - Flags: review?(LpSolit)
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?
Assignee: myk → romaia
Blocks: 242318
No longer depends on: 242318
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.
No longer blocks: 242318
Depends on: 242318
Target Milestone: --- → Bugzilla 2.22
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 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-
Attached patch v2 - romaia (obsolete) (deleted) — Splinter Review
Attachment #188450 - Attachment is obsolete: true
Attachment #191243 - Flags: review?
Attached patch v4-romaia - minor changes (deleted) — Splinter Review
fixing bitrot
Attachment #191243 - Attachment is obsolete: true
Attachment #191524 - Flags: review?(LpSolit)
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.
Attachment #191524 - Flags: review?(mkanat)
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 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-
Attachment #191243 - Flags: review?
Attachment #191524 - Flags: review?(LpSolit)
Version: unspecified → 2.21
Depends on: bz-postbugpm
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
QA Contact: mattyt-bugzilla → default-qa
Depends on: bz-bugwrite
No longer depends on: bz-postbugpm
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
Target Milestone: Bugzilla 3.2 → ---
Max, agree to mark this bug as WONTFIX? I see no benefit in merging post_bug.cgi and process_bug.cgi together.
Yes, agreed.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: