https://bugzilla.redhat.com/show_bug.cgi?id=1969450 --- Comment #9 from Martin Pitt <mpitt@xxxxxxxxxx> --- Thanks Ben for the detailed review! I a few quick comments/questions about some issues: > - It appears to me that the Release field, > Release: 1.20210908154854083687.pr65.33.gfb97935%{?dist} > does not follow one of the established “snapshot versioning” schemes Don't worry, this is just how a git commit through Packit-as-a-Service looks like. The actual fedora release will just be "1" or "2", or possibly "1.2" for a point release. > - You need > ExclusiveArch: %{nodejs_arches} noarch This sounds strange -- all of the js code is being run by a remote browser, not by node.js. So it may not make a practical difference, but it's still conceptually wrong IMHO. This is *not* a node.js package/library. > - Nothing owns /usr/share/cockpit. Indeed that's a bug in the cockpit package. It is currently owned by the "cockpit" metapackage, but that's a way too strong dependency. It should instead be owned by cockpit-bridge, as that's the correct dependency for third-party packages. As that also affects the dozen cockpit-{storage,packagekit,ostree,machines,podman} etc. packages, I rather propose that we fix that centrally in cockpit-bridge.rpm. WDYT? > - In general, when installing compiled/minified JavaScript, you must install the original unminified sources The guide says "included" -- it is included in the srpm, but certainly not in the binary rpm. That's exactly how every other language works, too -- we don't include C/Java/Rust sources into binary packages as well. The other paragraphs are indeed aimed at actual node-* libraries, it seems - but this isn't one. The other comments are quite clear and straightforward -- I'll fix them in the next round. -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=1969450 _______________________________________________ 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 Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
