Fedora Merge Review: desktop-file-utils http://cvs.fedora.redhat.com/viewcvs/devel/desktop-file-utils/ Initial Owner: rstrode
it's a little weird we BuildRequires: emacs. I'm not sure why we do that. We should probably minimally make it a Requires: and probably move the .el file to a subpackage.
well, it's only one file, let's not bother making the subpackage for now.
(and drop the Requires: entirely)
there's some cruft: # We don't want the vfs module yet /bin/rm -f $RPM_BUILD_ROOT%{_libdir}/gnome-vfs-2.0/modules/libmenu* /bin/rm -f $RPM_BUILD_ROOT%{_sysconfdir}/gnome-vfs-2.0/modules/menu-modules.conf I have no idea what that's about, but I'm dropping it.
We don't install any .desktop files from desktop-file-utils, so it doesn't make sense to call update-desktop-database from it.
So it looks like we have to keep the emacs buildrequires to generate the .elc file.
1) packaging guidelines suggests use of make as http://fedoraproject.org/wiki/Packaging/Guidelines#head-525c7d76890cb22df33b759c65c35c82bf434d2e 2) good to use %defattr(-,root,root,-) 3) Do you want to solve following rpmlint messages? desktop-file-utils.i386: W: obsolete-not-provided desktop-file-validator desktop-file-utils.src:16: W: unversioned-explicit-obsoletes desktop-file-validator 4) I saw new upstream release. you will like to update source. Update package. Better to provide new SPEC and SRPM links for this package before actually committing in CVS.
Do anyone care to provide updates here? I see so many co-owners for this package.
I've dropped the Obsoletes: desktop-file-validator That's just legacy cruft. desktop-file-utils is already in cvs. this review is just a post core merge review.
(In reply to comment #9) > desktop-file-utils is already in cvs. this review is just a post core merge review. What does this exactly means? This package hasn't been reviewed already, it may not meet the criteria for inclusion in fedora. * does desktop-mime-type.prov really belong to that package? Looks like it should better be in redhat-rpm-config. * there are emacs guidelines http://fedoraproject.org/wiki/Packaging/Emacs * the following comment is strange, certainly a leftover: # https://bugs.freedesktop.org/show_bug.cgi?id=12018 * please don't mention GNOME and KDE in description, better use freedesktop or the like. All the window manager in fedora I know about use the .desktop files to create their menu. * Is there a reason why %_smp_mflag isn't used? * the source archive timestamp isn't kept, it is too late now, but please keep it next time -rw-rw-r-- 1 dumas dumas 348871 Feb 11 2008 desktop-file-utils-0.15.tar.gz -rw-rw-r-- 1 dumas dumas 348871 Mar 4 2008 desktop-file-utils-0.15.tar.gz-old * source match upstream 2fe8ebe222fc33cd4a959415495b7eed desktop-file-utils-0.15.tar.gz
desktop-file-utils is already included in fedora. Feel free to commit fixes for minor packaging issues you find with it. The ACL should be open on the package.
Created attachment 320728 [details] file spec patch to implement request of the review Please review and tell me if you want the whole patch to be applied. If yes I'll apply and rebuild, if no, please apply what you want.
Everything looks good but removing the .prov file. If you think it should go in redhat-rpm-config or whatever, we should get it there first before taking it out here.
(In reply to comment #13) > Everything looks good but removing the .prov file. If you think it should go > in redhat-rpm-config or whatever, we should get it there first before taking it > out here. Please submit it to redhat-rpm-config then, I'd prefer if you do it, since redhat-rpm-config are not very responsive to me.
filed Bug 468701
Created attachment 449859 [details] spec cleanup halfline, I think we have now official emacs guidelines. I have used them and added emacs addons subpackages here. Can you please check attached patch if it looks good to commit so that this will get cleaned up?
looks fine, feel free to commit it.
Thanks. I have committed above patch and built new package desktop-file-utils-0.16-2.fc15