[Bug 456190] Review Request: dosemu - dos emulator

[bugzilla at redhat.com]

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 at v3.sk>  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.