https://bugzilla.redhat.com/show_bug.cgi?id=1229610 Jeff Backus <jeff.backus@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@xxxxxxxxxxxxxxxxx |jeff.backus@xxxxxxxxx Flags| |fedora-review? --- Comment #3 from Jeff Backus <jeff.backus@xxxxxxxxx> --- (In reply to Nikos Mavrogiannopoulos from comment #2) > Thank you for the review. Welcome! > > * Please get in touch with upstream and have them include the LICENSE file. > > Done. > https://github.com/gsauthof/doxy2man/issues/2 Thanks! Looks like upstream has already addressed the issue. > > * %build fails to honor applicable compiler flags. With qmake, you need to > > export the flags first. e.g.: > > export CFLAGS=$RPM_OPT_FLAGS > > export CXXFLAGS=$RPM_OPT_FLAGS > > qmake-qt5 > > Done. Looks good. Thanks! > > * Please do not hard-code directory names like you did with manpage.xsl on > > line 40. > > There is no other way for it. Anyway it is no longer needed in the spec > since the manpage is now upstream. Glad the point is now moot. > > * Please provide koji scratch builds for F21, F22, and Rawhide. > > F21: http://koji.fedoraproject.org/koji/taskinfo?taskID=10196035 > F22: http://koji.fedoraproject.org/koji/taskinfo?taskID=10195969 > rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=10196039 Builds look good. > > * SourceX entries need URLs. Source1 doesn't have a URL at all. For Source0, > > I've been pretty successful with the following GitHub URL scheme: > > Updated and simplified versioning by removing the hash. Looks good - and more legible. Thanks. > > * Please use %{name} anywhere you can, such as the URL and source file names. > > I'd prefer not. I believe the URL should be available directly to someone > reading the spec file without any need to replace parts to access it. About > the files I'd also prefer to have things simple. I'll only change that if > you insist for accepting the package. Fair enough. I won't push, as you are using macros for everything else. > > I'm happy to complete the review if you can address the above issues. I > > didn't get a chance to ensure that it works as advertised, but will do so > > when I get a chance. > > The simplest way to check it if it works to generate the manpages in > https://github.com/radcli/radcli (./configure && make && cd doc/man/) > > Updated URLs: > http://people.redhat.com/nmavrogi/fedora/doxy2man.spec > http://people.redhat.com/nmavrogi/fedora/doxy2man-0-1.20150624git.fc22.src. > rpm Thanks for addressing all of the issues I raised. Package looks good, though I do have one last request: please update to the latest version since it does contain a license file. Regards, Jeff -- 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 https://admin.fedoraproject.org/mailman/listinfo/package-review