Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=456190 --- Comment #44 from Lubomir Rintel <lkundrak@xxxxx> 2009-03-22 15:02:48 EDT --- Guys, please don't discuss the functional issues here, until the package is in Fedora. Once there will be a single official build of the package you'll agree on, then it will start to make sense to solve issues, not before then. It makes this review report a lot less legible for the reviewer, and it's increasingly hard to keep track of the packaging issues here. Derek, thank you for the participation though. When it comes to the compiler flags, I see them being properly used, since %configure macro sets them correctly. Probably past use of ./configure instead of %configure resulted into build w/o proper flags, but it all seems OK to me now. (Please be sure you understand it though). Justin, are there any examples of your work that could prove that you understand packaging well? Some informal reviews? I'll need to see it so that I can sponsor you. So here's first crack at the official review (sorry that it took so long): 1.) Source files Please comment on how did you get these: VCS checkout? Which revision? Source: %{name}-%{version}.tgz Full URL? Source1: %{name}-freedos-bin.tgz Original work? Source2: %{name}.desktop Isn't etc/dosemu.xpm from the source tarball sufficient? Source3: %{name}.xpm 2.) FreeDOS image I don't believe this is formally allowed (shipping binaries), though other packages do this (say, qemu includes bochs bios image). I'll ask on packaging list how to deal with this and let you know. It is also probably illegal (if it's GPL) to distribute this without source code, so at the very least, please add also the source code package and, as always, please comment as heavily as you can :) In case we won't be able to this -- do you know if it's very hard to build FreeDOS kernel image and cross-build the basic utilities (at least COMMAND.COM) with the tooling currently in Fedora? Source1: %{name}-freedos-bin.tgz 3.) Please use make flags. Please add %{_smp_mflags} after make, unless it breaks built. If it does, please add a comment instead. See: http://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make 4.) Legibility Please try not to cross 75 or 80 column boundary: chmod 755 $RPM_BUILD_ROOT%{_datadir}/dosemu $RPM_BUILD_ROOT%{_datadir}/dosemu/drive_z $RPM_BUILD_ROOT%{_datadir}/dosemu/drive_z/doc/exe2bin That is better written as: chmod 755 $RPM_BUILD_ROOT%{_datadir}/dosemu \ $RPM_BUILD_ROOT%{_datadir}/dosemu/drive_z \ $RPM_BUILD_ROOT%{_datadir}/dosemu/drive_z/doc/exe2bin 5.) Group RPMLint complains: dosemu.i586: W: non-standard-group Game/Emulator Indeed, this is not a standard group. Pick one from /usr/share/doc/rpm*/GROUPS. I believe the right one is "Applications/System". Hint: This is not used for anything, so does not matter at all. It can even be omitted since Fedora 10. 6.) .desktop entry Please set the back to Category=System;Emulator from Category=Game;Emulator, which is the only right value in this case. I believe we've have communicated this with Andrea via private mail back then. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review