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)
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: jeff.hedlund, Assigned: romaia)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
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•20 years ago
|
Severity: normal → enhancement
OS: other → All
Hardware: Other → All
Assignee | ||
Comment 2•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #188450 -
Flags: review?
Updated•20 years ago
|
Attachment #188450 -
Flags: review?(LpSolit)
Comment 3•20 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•20 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•20 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•19 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•17 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•17 years ago
|
||
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.
Description
•