https://bugzilla.redhat.com/show_bug.cgi?id=1410901 Randy Barlow <randy@xxxxxxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jeremy@xxxxxxxxxx Flags| |needinfo?(jeremy@xxxxxxxxxx | |) --- Comment #1 from Randy Barlow <randy@xxxxxxxxxxxxxxxxxxxxx> --- 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. Should ====== * You should BuildRequires: python2-devel. I'm actually slightly surprised the build works without it… * 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. * I think fedora packages are supposed to use /usr/bin/python2 instead of /usr/bin/env python * 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. Optional ======== * I don't think you need to BuildRequires systemd. Is systemd-devel needed for the %{_unitdir} macro to exist? * You could make the description be a global so you don't have to repeat 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). * rpmlint wants irc to be capitalized. * It would be good to write a manpage for fmn-createdb. 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. -- 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 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx