Closed
Bug 71552
Opened 24 years ago
Closed 24 years ago
Remove oldemailtech from Bugzilla
Categories
(Bugzilla :: Bugzilla-General, defect, P1)
Bugzilla
Bugzilla-General
Tracking
()
RESOLVED
FIXED
Bugzilla 2.14
People
(Reporter: justdave, Assigned: jacob)
References
Details
Attachments
(4 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details |
I could swear we had a bug for this already, but I can't find it, so I'm making one.
Reporter | ||
Comment 1•24 years ago
|
||
Making this a 2.14. It's not exactly security, but bugzilla.mozilla.org can't easily cvs update again until this is yanked. Regenerating the shadow directory (the ASCII copies of all the bugs in ${BUGZILLA_HOME}/shadow that oldemailtech uses to make the diffs it mails out) on running checksetup.pl takes approximately 7 hours to run on b.m.o with 71000 bugs.
Target Milestone: --- → Bugzilla 2.14
Assignee | ||
Comment 6•24 years ago
|
||
What exactly is the purpose of having it regenerate the shadow files in checksetup? I'd imagine it has something to do w/making sure that new fields get added to the shadow files at update instead of at the first change of the bug (being that the files already exist simply by the fact that the bug was changed). Being that oldmail is on it's way out, wouldn't it be possible to simply pluck that routine from checksetup.pl for 2.14 (possibly the first thing done in 2.13)? We could then save the real work of removing oldmail for 2.16. Dont' get me wrong, I have no love for the oldmail system and would prefer to see it gone ASAP, but it just seems like it be better to wait for 2.16.
Comment 7•24 years ago
|
||
Marking this dependent on a bug that says emailsuffixes and newemailtech are mutually exclusive. I'm not sure if that will disappear along with this, but we can't break Bugzilla features.
Depends on: 61824
Assignee | ||
Updated•24 years ago
|
Blocks: emailprefs
Reporter | ||
Updated•24 years ago
|
Assignee | ||
Comment 8•24 years ago
|
||
OK, I can take this one and make it my #1 priority... I don't have a whole lot going on this week, so it should be done very soon. What I have on my list of things to do: * Remove code that supports oldmail from: - userprefs.cgi - processmail - checksetup.pl (namely, processmail regenerate) * Remove the following params: - newemailtech - newemailtechdefault - changedmail * Add a routine to checksetup to remove no longer needed stuff: - shadow/* - profiles.emailnotification - profiles.newemailtech Known issues: * Currently when somebody vists userprefs.cgi?bank=diffs for the first time it retrieves their current setting from profiles.emailnotification and if it's ExcludeSelfChanges it flags that option in the profiles.emailflags field. While it's possible to do that in checksetup.pl before removing the field, I personally don't think it's worth it. If I'm wrong, I can add that functionality to the checksetup.pl routine. * When removing a param from defparams.pl, it never removes it from data/params... probably not a big deal, but worth mentioning I'd also like an opionion on how to check for the existance of oldmail... Should it be if the shadow directory exists, or if profiles.newemailtech is in the DB?
Assignee: tara → jake
Priority: -- → P1
Comment 9•24 years ago
|
||
I suggest you don't have just one check for old emailtech. Remove shadow if shadow exists, remove the params if they exist, etc. That way you can cope with various situations like the script dying half way through.
Assignee | ||
Comment 10•24 years ago
|
||
Assignee | ||
Comment 11•24 years ago
|
||
Lot's of "-" in this patch :) Things changed since my last comment: * Discovered that Param("prettyasciimail") was no longer needed * Added globals.pl and editusers.cgi to the list of files to remove code from Notes about this patch: * Added support for data/nomail to the newemailtech code * There are almost no whitespace changes even though I removed "if" statements and came accross other code that isn't use "4 space indenting". This was mostly to make it easier to see what changed. * I removed the "CC: %cc%" line from the newchangedmail default param. CC isn't used by newemailtech anyway. I did leave the code in processmail otherwise the existance of "%cc%" in the param will cause processmail to fail * I'm 95% sure that the Lock and Unlock subs were there because of the file level diffing, so I removed them. * Rather than simply removing the "regenerate" option from processmail, I changed it so it says "Regenerating is no longer required or supported" * In userprefs.cgi I removed some (all?) of the "NEW!!" indications.
Comment 13•24 years ago
|
||
No. Landfill is only for red-haired people with green eyes. Silly. Okay, patch is up. Let the testing commence.
Comment 14•24 years ago
|
||
I strongly suggest putting the whitespace changes in, so you don't end up with a file having illegible indentation. "cvs diff -b" can be used to show the differences without seeing the annoying whitespace-only changes.
Comment 15•24 years ago
|
||
While you are in the email system, any chance of you making CC field diffs a bit more intelligent? It would be _really_ handy for bugmail just to say who had been added and who had been removed. If this is too complex, can you at least change the code order so CC field changes appear at the bottom of bugmail? Gerv
Assignee | ||
Comment 16•24 years ago
|
||
Assignee | ||
Comment 17•24 years ago
|
||
Gerv... I do plan on changing that behaviour at some point, but it's not part of this bug. Bug 61015 is currently flagged for 2.16. I have some ideas on how to do that differently, but think that focusing on getting 2.14 out the door is the higher priority right now... ;)
Assignee | ||
Comment 18•24 years ago
|
||
Assignee | ||
Comment 19•24 years ago
|
||
I discovered that I missed a routine (user creation) in editusers.cgi... "patch v2" also contains whitespace changes...
Reporter | ||
Comment 20•24 years ago
|
||
Reporter | ||
Comment 21•24 years ago
|
||
patch v2 has been running on bugzilla.syndicomm.com for two weeks now and we haven't even noticed the difference. Which is a good thing for this particular patch. :-) r= justdave Although I think we ought to add the code to remove the params, that could probably be another bug.
Assignee | ||
Comment 22•24 years ago
|
||
Checked in. Bug 82497 has been created for the removal of the stranded params.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 23•23 years ago
|
||
Moving to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: Bugzilla 2.11 → unspecified
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•