[Bug 578290] Review Request: mj - Mah-Jong program with network option

[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=578290

--- Comment #6 from Göran Uddeborg <goeran at uddeborg.se> 2010-04-10 14:01:57 EDT ---
Thanks a lot for your review, or pre-review strictly speaking.  This is only my
second package for Fedora so I'm new at this myself.

> I only get two errors from rpmlint:

> > mj.src: W: no-cleaning-of-buildroot %install

> > mj.src: W: no-buildroot-tag

> Could you take a look at that?

As I understand it, none of these have been needed since F-10
(https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag).  I'm only
planning to ask for this to be released for F-13.  Thinking about it, that
actually means I could simplify a bit more, and remove the %clean section too.

> 'mj' is a *very* short name.

> Do you think upstream would
> be willing to change name to e.g. mahjong-1.10
> or mahjongg-1.10?

It is indeed short.  It will hardly hurt to discuss it with upstreams.  I'll
send him a letter and ask.

Including a version number to distinguish two completely different programs
like Mah-Jong from mj and the solitaire Mah-Jong from gnome-games might not be
ideal.  Version numbers in command names are typically used when you explicitly
want to use a particular version, like for automake and python.

Until upstreams changes, I guess the rules from the naming guidelines about
following upstreams prevail and I keep the name?  Or should I change the naming
specifically for Fedora?

> The application is written in C but uses neither
> $RPM_OPT_FLAGS nor %{optflags}

Ah!  Forgot that!  Thanks, I'll fix.

> … should also BuildRequire desktop-file-utils (mj.spec doesn't)

It's indirectly required.  But it's probably better do require it explicitly. 
I'll add it.

> mj.spec contains
> two instances of %define. Is that needed?

Nope, old habits die hard.  I'll fix it.

> Consider using
>   cp -p ../tiles-v1/tong* .

Good point, I will.

> But then I suppose the tiles-v1/ directory should be removed from
> the source package since otherwise the source package will contain
> tiles which are not GNU GPL.

Are you saying I should modify the tar file?  I thought that was something we
didn't do.  And since I don't actually *use* the restricted tiles, they are not
part of the binary package, I thought this was ok.

But I may be wrong.  Should I bother fedora-legal once more about this, maybe?

> … except lazyfixed.c, lazyfixed.h, vlazyfixed.c, and
> vlazyfixed.h which refer to
>   GNU Lesser General Public License (any version).
> Is that a problem?

Obviously not for distributability.  Reading the first answer of
https://fedoraproject.org/wiki/Licensing/FAQ#How_should_I_handle_multiple_licensing_situations.3F
I believe that the license tag should only say the stricter license (GPL) in
this case.  Do you agree?  Or should I ask fedora-legal about this (too)?

> … the author claims moral rights.
> Does that have any effect?

I can't imagine how it could make a difference if the author "asserts" a right
he can't surrender anyway.  But I see that Mamoru Tasaka has already brought
that up with Fedora Legal while I was writing this reply, wo I guess we will
have an answer.

> I am unable to test PPC. What shall I do?

I *expect* it to build on PPC.  Later when I can start builds via koji, I can
verify that it indeed does.  If there is a way to verify earlier, I would like
to know just as much as you.

> How can I find out if one needs to require
> hicolor-icon-theme-0.10-6.noarch and
> policycoreutils-2.0.62-12.14.fc11.x86_64 ?

/usr/share/man/man1 is owned by filesystem too, so policycoreutils should not
be needed.

There seems to be plenty of packages that places things in
/usr/share/icons/hicolor/32x32/apps/, but much fewer require
hicolor-icon-theme.  So I guess it's default.  But maybe all those packages
should have required it?

> Why are man pages not user writable?

It's an upstreams decision.  The Makefile.in in the tar file sets them to
read-only for all.  Is this something I should change as packager?  Is there
any rule that files like man pages should be user writable?  There are rules
that the permissions should be set "properly".  But it's not clear to me that
r--r--r-- is less proper than rw-r--r-- for a manual page.  Do I miss
something?

I've made an updated version including those things that were clear:

SPEC: ftp://ftp.uddeborg.se/pub/mj/mj.spec
SRPM: ftp://ftp.uddeborg.se/pub/mj/mj-1.10-3.fc13.src.rpm

-- 
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.