Fedora Merge Review: mutt http://cvs.fedora.redhat.com/viewcvs/devel/mutt/ Initial Owner: mlichvar
This package is in very good shape! I have some comments though: * maybe you could use gpgme? * also maybe it could be nice to enable mixmailer the day it enters fedora * the lynx BuildRequires in my opinion deserves a comment * I think that htmlview would be a better Requires than webclient, that way mutt may be installed without webclient and at the same time there is more chance that an html viewer will be selected if one is installed * I am not completly convinced that it is right to have urlview shipped with mutt. In the mutt documentation it is flagged as being external. One could imagine people wanting urlview without mutt. And there could also be people wanting mutt without urlview, although I wouldn't find it abnormal if mutt depended on urlview * the second paragraph of the %description seems a bit strange to me. It seems more confusing than explaining to me, and it seems to describe mutt more or less as the default text client mail which seems a bit hard for newcommers to me -- although I don't know what alternatives exist. * Maybe /etc/Muttrc may be made %config and not %config(noreplace) since there is Muttrc.local? * The XTERM path in url_handler.sh is wrong. I don't think xterm should be a dependency of urlview, since it would imply getting in X libs which shouldn't be required in case of mutt/urlview in my opinion. * What do you think about giving as a Requires of urlview an application for all of the categories (https, http, mailto, gopher, ftp)? It is not necessarily a good idea, but maybe something to think about.
(In reply to comment #1) > * also maybe it could be nice to enable mixmailer the day it enters > fedora sorry it is mixmaster, not mixmailer...
Thanks for looking at this. (In reply to comment #1) > * maybe you could use gpgme? What would be the advantage of including support for gpgme? It brings additional runtime dependencies, so it has to be really useful :). > * also maybe it could be nice to enable mixmailer the day it enters > fedora When mixmaster is in Fedora, I'll consider it. > * the lynx BuildRequires in my opinion deserves a comment Ok. > * I think that htmlview would be a better Requires than webclient, > that way mutt may be installed without webclient and at the same > time there is more chance that an html viewer will be selected > if one is installed Hm, htmlview depends on redhat-menus. I'd prefer to remove the webclient dependency and not depend on any external application beside sendmail and urlview (and perl from smime_keys). If user hasn't installed a browser, I think it's ok that s/he won't be able to start a browser from mutt. And maybe even the sendmail dependency can be removed since mutt has an smtp client compiled in. > * I am not completly convinced that it is right to have urlview > shipped with mutt. In the mutt documentation it is flagged as > being external. One could imagine people wanting urlview without > mutt. And there could also be people wanting mutt without urlview, > although I wouldn't find it abnormal if mutt depended on urlview Ok, good point, urlview should be packaged separately. > * the second paragraph of the %description seems a bit strange to > me. It seems more confusing than explaining to me, and it seems > to describe mutt more or less as the default text client mail which > seems a bit hard for newcommers to me -- although I don't know what > alternatives exist. Ok, will be removed. > * Maybe /etc/Muttrc may be made %config and not %config(noreplace) > since there is Muttrc.local? I'm not sure it's a good idea, I'd prefer to keep it as noreplace. Users can have /etc/Muttrc configured and ignore /etc/Muttrc.local. > * The XTERM path in url_handler.sh is wrong. I don't think xterm > should be a dependency of urlview, since it would imply getting in > X libs which shouldn't be required in case of mutt/urlview in my > opinion. I'll fix the path. If xterm isn't installed, the application will be started in current terminal, so no need for the dependency. > * What do you think about giving as a Requires of urlview an > application for all of the categories (https, http, mailto, > gopher, ftp)? It is not necessarily a good idea, but maybe > something to think about. I'd prefer to not depend on anything, but I'm open to suggestions :).
Please don't introduce any dependency which involves other (unneeded) packages like at htmlview. The package htmlview is just big overkill if you simply want to run a server system including mutt with less as possible packages.
Is there any big advantage by introducing gpgme support? It just seems like another GPG implementation providing more or less exactly the same which gnupg currently does...sorry. Corrections are cheerfully accepted ;-)
(In reply to comment #4) > Please don't introduce any dependency which involves other (unneeded) packages > like at htmlview. The package htmlview is just big overkill if you simply want > to run a server system including mutt with less as possible packages. I proposed htmlview instead of webclient. webclient may be much bigger than htmlview. redhat-menus depends only on desktop-file-utils which doesn't depend on many things.
(In reply to comment #3) > Thanks for looking at this. > > (In reply to comment #1) > > * maybe you could use gpgme? > > What would be the advantage of including support for gpgme? It brings additional > runtime dependencies, so it has to be really useful :). I don't know at all but there is one muttrc option that can only be set with gpgme: crypt_use_pka > > * I think that htmlview would be a better Requires than webclient, > > that way mutt may be installed without webclient and at the same > > time there is more chance that an html viewer will be selected > > if one is installed > > Hm, htmlview depends on redhat-menus. I'd prefer to remove the webclient > dependency and not depend on any external application beside sendmail and > urlview (and perl from smime_keys). If user hasn't installed a browser, I think > it's ok that s/he won't be able to start a browser from mutt. Right. > And maybe even the sendmail dependency can be removed since mutt has an smtp > client compiled in. That would allow to remove the Provides of msmtp, ssmtp and esmtp, that would be nice. > Ok, good point, urlview should be packaged separately. Right. > > * Maybe /etc/Muttrc may be made %config and not %config(noreplace) > > since there is Muttrc.local? > > I'm not sure it's a good idea, I'd prefer to keep it as noreplace. Users can > have /etc/Muttrc configured and ignore /etc/Muttrc.local. This would allow to have a 'vendor/packager' defaults file that is under the control of the packager and allow to change things when needed to. > I'll fix the path. If xterm isn't installed, the application will be started in > current terminal, so no need for the dependency. Right. > I'd prefer to not depend on anything, but I'm open to suggestions :). If mutt depends on urlview, right it is better not to depend on anything. But then urlview is sort of non-functional, though this is not necessarily wrong since it allows to avoid dependencies which is right for a program that allows for a choice. If mutt doesn't depends on urlview, then I'd say it depends on you.
(In reply to comment #6) > I proposed htmlview instead of webclient. webclient may be much > bigger than htmlview. redhat-menus depends only on desktop-file-utils > which doesn't depend on many things. Actually this is wrong, redhat-menus also depends on the low level gnome stuff so htmlview isn't much better than webclient.
A review request for urlview filed at bug #245951.
I have saw, that the most current version is 1.5.16, If you may update your package to this version, you may write Conflict: mutt < 1.5.16 in the urlview package.
Urlview is removed from mutt-1.5.16-2.fc8. Description and requires should be also fixed. I didn't remove the noreplace flag for /etc/Muttrc as I still think it's better to allow the users to have a different system-wide config without forcing them to undo the settings provided by the package.
The only issue I see that isn't addressed is the gpgme issue. There is one muttrc option that can only be set with gpgme: crypt_use_pka Therefore shouldn't gpgme support be built in?
Could you apply this patch? http://www.spinnaker.de/mutt/compressed/ It's a great disk space saver when dealing with mail archives in mbox format.
The patch author admits that it isn't stable still. So I am not sure that it should be in fedora.
If you're referring to the line "I fear that there are still some bugs or inconsistencies in this code (that's why Thomas Roessler didn't include it into the Mutt distribution yet)." then I assure you, it's been on that website for years. I've been using this patch since mutt-1.4 with no ill effects.
Any estimate how many users will use PKA support? I don't even know what it is good for. ;) Enabling gpgme will add more than 10MB to mutt requirements. As for the compressed folders patch, is there a more specific reason why it's not included upstream yet? I'd rather not include any non-trivial patches that will need to be maintained forever.
Regarding the PKA support, my opinion is that you are the best one to decide. Maybe you could reconsider your opinion if users ask for it but in the mean time it is perfectly right, in my opinion, not to enable it. The compressed folder patch should go upstream, in my opinion before going in fedora. So things are right here too. Source match upstream 49387458be0cb52b85ae0d73af699aae mutt-1.5.17.tar.gz timestamp isn't kept: -rw-rw-r-- 1 dumas dumas 3572651 nov 2 13:22 mutt-1.5.17.tar.gz -rw-rw-r-- 1 dumas dumas 3572651 nov 1 21:05 mutt-1.5.17.tar.gz-orig it is too late now, but please keep it next time. APPROVED.
Thanks. The timestamp of the file I've uploaded was correct though. Maybe a problem in the upload rule in Makefile.common or the upload.cgi page? Looking at last three of my updates, all have wrong timestamp.
(In reply to comment #18) > Thanks. > > The timestamp of the file I've uploaded was correct though. Maybe a problem in > the upload rule in Makefile.common or the upload.cgi page? Looking at last three > of my updates, all have wrong timestamp. It works for me. Be warned that once there is a file with a given timestamp it cannot be changed. Anyway maybe you could close the bug?