https://bugzilla.redhat.com/show_bug.cgi?id=843997 --- Comment #4 from Ryan Curtin <ryan@xxxxxxxxxxxx> --- Hello there, Thank you for the numerous comments. It appears I have many things to learn before becoming a sponsored packager! I have unofficially reviewed two packages and in doing so have learned much about the Fedora packaging guidelines which I thought I knew... but apparently did not. https://bugzilla.redhat.com/show_bug.cgi?id=820115 https://bugzilla.redhat.com/show_bug.cgi?id=843678 > Not covered by any guidelines, but in many cases the leading article is superfluous and doesn't increase conciseness. > > Summary: Scalable, fast C++ machine learning library You are right; fixed. I also fixed the group issues which were mentioned; the libraries and headers now reside in 'System Environment/Libraries', and the executables live in 'Applications/Engineering'. If those choices are wrong, let me know and I will have it fixed quickly. I commented the entire spec file more heavily, including better reasons for each of the patches and justification for versioned dependencies. > %_mandir implicitly is marked as documentation. Also, the gzip compression may change or be disabled within a buildsystem, so using wildcards can be beneficial. > > %{_mandir}/man1/allknn.1* Neat, I did not know that. I've fixed that also. The other issues you've mentioned have also been fixed. > Since you're upstream, I'd recommend considering a renamal of the binaries to have, say, a ml_ prefix. I am not sure what we want to do upstream yet, but for now in this package I rename all the binaries to mlpack_$name (i.e. mlpack_radical) and rename the man pages correspondingly using a little bash for loop. I think that should resolve the ambiguity for now, and that may be the long-term upstream solution too. > An honest attempt at trying to review your own package could be enlightening. You were right; I went through all the MUST/SHOULDs and did not find any others which are a problem (other than the ones you pointed out). I then rebuilt on koji successfully (and tested the executables locally): http://koji.fedoraproject.org/koji/taskinfo?taskID=4344100 The updated spec: http://www.mlpack.org/files/mlpack.spec The updated srpm: http://www.mlpack.org/files/mlpack-1.0.1-3.fc17.src.rpm If there is anything else I've missed, let me know and I will fix it quickly. Thanks! -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review