https://bugzilla.redhat.com/show_bug.cgi?id=1590425 Cole Robinson <crobinso@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@xxxxxxxxxxxxxxxxx |crobinso@xxxxxxxxxx Flags| |needinfo?(dinechin@redhat.c | |om) --- Comment #10 from Cole Robinson <crobinso@xxxxxxxxxx> --- Using the SRPM URL: and Spec URL: format in Comment #3 helps simplify the review process, fedora-review tool can just be pointed at the bug. So for next posting please provide that as well * The kata-runtime binary should be in /usr/bin, which is where upstream puts it. I don't see any reason to deviate * godocs/golicense/files: only a single LICENSE file ends up in the rpm. I don't think the godocs/golicense stuff is playing well with the %files section, but I'm not sure. either way, I don't think we need to put anything more than the top level LICENSE and README.md into the RPM. The LICENSEs are all the same apache version, and most of of the other docs seem fine to just exist in upstream git. * The BuildRequires: compiler(go-compiler) is not required, go-rpm-macros does that for us * The comment about a 'rediculous' name from goname is not necessary. IMO it's self evident why goname does not apply here, it doesn't need the commentary Trimmed fedora-review output below, with some comments. [ ]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/defaults * Yes, seems like this should be a %dir in the %files list. This is a non-standard FHS directory I believe, but it will take work with kata upstream to correct it, so it should stay for now IMO> [ ]: %build honors applicable compiler flags or justifies otherwise. * We talked about this offline. Use of %gobuild is not easy in this package because 'make' handles a bunch of other stuff. So distro go build flags aren't making it into the build. Next release of go-rpm-macros should have a %gobuildflags macro that will help here, but it isn't available yet. Christophe, please add a comment to that effect in the spec, so we don't forget, and optionally link to the go-rpm-macros commit: https://pagure.io/go-rpm-macros/c/67b4fbbbfce0986ac46cd1329bf85a18ea7a43d2?branch=master [ ]: Package is not known to require an ExcludeArch tag. * ExcludeArch is necessary as dependent kata components do not build on 32 bit archs. This is a known issue that isn't kata-runtimes fault. Doesn't block this review [!]: Reviewer should test that the package builds in mock. * worked fine for me [!]: Uses parallel make %{?_smp_mflags} macro. * indeed, spec should use %make_build and %make_install macros in place of the direct 'make' calls. But if this causes build issues, just drop them and mention it in the next submission Rpmlint ------- Checking: kata-runtime-1.8.2-2.fc32.x86_64.rpm kata-runtime-debuginfo-1.8.2-2.fc32.x86_64.rpm kata-runtime-debugsource-1.8.2-2.fc32.x86_64.rpm kata-runtime-1.8.2-2.fc32.src.rpm kata-runtime.x86_64: W: no-manual-page-for-binary kata-collect-data.sh 4 packages and 0 specfiles checked; 0 errors, 1 warnings. * This issue can be safely ignored. I think we may not want to ship this script, or move it elsewhere, but we can figure it out after package import -- 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 Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
