https://bugzilla.redhat.com/show_bug.cgi?id=1410901
Jeremy Cline <jeremy(a)jcline.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|needinfo?(jeremy(a)jcline.org |
|) |
--- Comment #2 from Jeremy Cline <jeremy(a)jcline.org> ---
Spec URL:
https://jcline.fedorapeople.org/python-fmn.spec
SRPM URL:
https://jcline.fedorapeople.org/python-fmn-1.1.0-1.fc25.src.rpm
(In reply to Randy Barlow from comment #1)
A few things, some must some optional:
Must
====
* I think this package does Provide those packages it obsoletes and should
be marked as such. For one, the "import fmn.whatever" statements will keep
working, but for two the Fedora release upgrade path will be broken without
it.
Thanks, I was not sure about this, but I think you're right. I've added the
Provides statements.
Should
======
* You should BuildRequires: python2-devel. I'm actually slightly surprised
the build works without it…
Fixed.
* The cp command in the install section won't preserve the
timestamp. You
could use the -a flag to get cp to preserve it for you.
Also fixed.
* I think fedora packages are supposed to use /usr/bin/python2
instead of
/usr/bin/env python
I changed upstream to generate the /usr/bin entry with setup.py
* Does alembic.ini really need to be 640? It didn't appear to
have secrets
in it at first glance, and it's installed to /usr/share (which means users
shouldn't be editing it anyway). It should probably be 644.
Nope, that's what I changed and I apparently forgot to rebuild the SRPM when I
uploaded it.
Optional
========
* I don't think you need to BuildRequires systemd. Is systemd-devel needed
for the %{_unitdir} macro to exist?
The systemd package provides the macro, but I don't know why I thought I needed
systemd-devel. I've removed that requirement.
* You could make the description be a global so you don't have to
repeat it.
I never remember how to make this work correctly so I just copy it :(
* It would be good to work on the upstream setup.py so that it
installs the
executable for you (so you don't have to do it manually in the install
section).
Done
* rpmlint wants irc to be capitalized.
Done
* It would be good to write a manpage for fmn-createdb.
Issue filed:
https://github.com/fedora-infra/fmn/issues/161
Also, fedora-review got mad that the spec URL and the SRPM don't
match. I
think it tested the spec inside the SRPM rather than the URL one.
I've been really careful this time to not tweak the specfile without rebuilding
the SRPM.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component