https://bugzilla.redhat.com/show_bug.cgi?id=1375765 Randy Barlow <randy@xxxxxxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ | |needinfo?(spacewar@xxxxxxxx | |m) --- Comment #6 from Randy Barlow <randy@xxxxxxxxxxxxxxxxxxxxx> --- (In reply to Eric Smith from comment #5) > Spec URL: https://fedorapeople.org/~brouhaha/yosys/yosys.spec > SRPM URL: > https://fedorapeople.org/~brouhaha/yosys/yosys-0.6.0-2.20160923git8f5bf6d. > fc24.src.rpm > > Note that no action was taken for the following: Thanks for pointing these out! > * bundled viz.js - The file viz.js isn't bundled into the generated RPMs, > so the bundling policy, which is based primarily on security concerns, is > not applicable. In particular, following the bundling policy would require a > "Provides: bundled(viz.js)=0.0.3", but that would clearly be wrong, as the > package doesn't provide it. I'm convinced and I agree with you now. > * /usr/share/yosys/python3/smtio.py - Not moved. There is considerable > precedent for application-specific Python files to be in /usr/share/%{name}, > e.g., firewalld, hplip, qtcreator, setroubleshoot, virt-manager, etc. I've > reviewed FHS 3.0 and am not convinced that having application-specific > Python files in /usr/share/%{name} actually contravenes any FHS 3.0 > requirement. I still think this *should* be moved, but due to your citations we can keep it the way it is. > * tarball without URL - No actual problem. Perhaps was triggered by outdated > URL for Debian pool. OK. FYI, the current URL is also 404, but I see the problem with the disappearing files. I recommend working with upstream to get that man page included there so we don't depend on Debian's man pages (and then Debian can also get them from upstream!) > * %check tests directory - No actual problem. Tests are present and are > correctly tested in %check section. OK > These requested changes have been made: > > * license tag - Changed to include additional licenses. I read the issue you filed upstream, and it seems that some of the licenses included in the current spec file only apply to tests and not to the distributed package. I recommend trimming the licenses down to what is in the distributed code, but I'm not sure whether the license field should be about the entire source tree, or just the bits we distribute. I'll leave that decision up to you. > * /usr/share/yosys/python3/smtio.p - Had previously marked executable to fix > rpmlint error. Removed the chmod, so now rpmlint reports that error. Which > is better, having the rpmlint error, or having the file unnecessarily marked > as executable? Since it contains no main program, I felt that having it > marked executable was inconsequential. It doesn't make sense for the file to be executable so I think the current state is better. If you want to silence rpmlint, you can drop the #!/usr/bin/python3 from the top of the source file so rpmlint doesn't think it's a script. It isn't a script anyway, so that line really isn't necessary. > * versioned dependency in subpackages: > - Subpackage doc dependency on main package *removed*, as docs can stand > alone. > - Subpackage devel dependency on main package made arch-specific. > * /usr/share/yosys is architecture-independent: > - Moved /usr/share/yosys to noarch subpackage. > - Main package made dependent on -share subpackage. The share subpackage is nice! There's a new message from fedora-review with the current spec that says that there is a large amount of data (3143680 bytes) in /usr/share, and it suggests moving that data to a noarch subpackage. Since you now have a -share package, perhaps that would be a nice improvement. One other message from fedora-review that you might consider fixing - it says that gawk isn't needed in BuildRequires. You can change that or not at your option. There is one thing you do need to fix for sure: [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/doc/yosys randy: You can probably change the doc files section to just have %{_docdir}/%{name} Once you have that directory ownership fixed let me know! -- 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