https://bugzilla.redhat.com/show_bug.cgi?id=1175023
--- Comment #20 from Gerald Cox <gb...@bzb.us> --- (In reply to Jerry James from comment #19) > Issues, in no particular order: > > - I am very uncomfortable with the way the source is handled. I see the > comments above about this issue, but I still don't understand why there is > a > problem with the official upstream URL: > > https://github.com/spr/Oggify/archive/v2.0.7.tar.gz > > What is the problem with using that? Jerry, I'm open to suggestions. The problem is that the way github packages the archives. It doesn't populate the submodules (in this case tag_wrapper). You have to manually: submodule init and update to get it included, then re-create the archive. It's a known issue, people have been complaining, but it still isn't fixed. I searched and searched and the only way I could find to do it was to copy an approach used by another Fedora package (comment #5). I don't particularly like it either, it's kind of a pain. You'd think that there would be a more elegant way to handle, but I haven't been able to find it. > > - Add "%dir %{python2_sitelib}/%{name}/plugins" to %files OK, no problem. > > - The plugins invoke %{_bindir}/nice, so "Requires: coreutils" is technically > needed, although possibly a bit silly. Yes, encountered that when I build for COPR. > > - Neither the %clean script nor the %defattr in %files is necessary. Please > remove both (unless you plan to build for EPEL5, in which case you will > have > to make a few other changes to the spec file, anyway). Thanks, I will remove. > > - Consider adding "%doc README.md" to %files. I think it contains some > useful > information. Will do. > > - I notice that python-mutagen is a BuildRequires, but not a Requires. Is > that intentional? On a previous review, I was told if something was listed in BuildRequires, that the Requires are automatically provided, and to not double-list. Is that not correct? > > - There is no %check script. Is there a simple test you could perform just > to > verify basic functionality is working? > > I'll look into that and provide if I can find something. Thanks for your input. I'll make the changes listed above. -- 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@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
