Closed Bug 71552 Opened 20 years ago Closed 19 years ago

Remove oldemailtech from Bugzilla

Categories

(Bugzilla :: Bugzilla-General, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Bugzilla 2.14

People

(Reporter: justdave, Assigned: jacob)

References

Details

Attachments

(4 files)

I could swear we had a bug for this already, but I can't find it, so I'm making 
one.
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
this is a test...
helping endico test
Testito.  Looks good so far, altho the vers is still 2.11....
i didn't follow instructions the first time, commenting again
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.
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
Blocks: 28458
Blocks: emailprefs
Blocks: 53044
No longer blocks: emailprefs
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
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.
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.
Status: NEW → ASSIGNED
Keywords: patch, review
Can this be tested on landfill?
No.  Landfill is only for red-haired people with green eyes.

Silly.

Okay, patch is up.  Let the testing commence.
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.
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
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...  ;)
I discovered that I missed a routine (user creation) in editusers.cgi...
"patch v2" also contains whitespace changes...
Blocks: 59349
No longer depends on: 61824
Blocks: 61824
Blocks: 78408
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.
Checked in.  Bug 82497 has been created for the removal of the stranded params.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
No longer blocks: 53044
Moving to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: Bugzilla 2.11 → unspecified
No longer depends on: 751809
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.