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 #46 from Justin Zygmont <solarflow99@xxxxxxxxx> 2009-03-22 19:29:45 EDT --- (In reply to comment #44) > 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. ok, thanks for letting us know. > 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). Thats good to know because I didnt see any place where opt_flags was used. > 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. ok, I am about to do this, i'll let you know. > 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: You mean comment where I got it in the spec file? > 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 I think I got that idea from the dosbox spec file, I can change it if necessary. > 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. ok, I guess I overlooked it as shipping binaries because it was just a freedos image. I hope it will be ok. > 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 :) ok, I guess the source URL in the comments. > 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 hmm, I guess that would be the ideal way, but the freedos kernel image is trimmed and customized a bit, i'd hate to have to go down that route just for a replaceable image. At this point I dont know what it takes to build that. This .tgz file is not something thats available on the freedos site, for the source code, all they have is a large ISO image. > 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 ok thanks. Done. > 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 All done. I just have a URL that is long, should I break that up too? > 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. Yes, thats done. I have also changed "linux" to Linux" :) The summary is accurate though, dosemu doesn't build on other platforms, there used to be a port to netbsd but that was a long time ago. I have uploaded the latest RPM packages and spec file here: http://jzygmont.fedorapeople.org/dosemu.spec -- 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