https://bugzilla.redhat.com/show_bug.cgi?id=1410901 Jeremy Cline <jeremy@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(jeremy@xxxxxxxxxx | |) | --- Comment #2 from Jeremy Cline <jeremy@xxxxxxxxxx> --- 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 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx