https://bugzilla.redhat.com/show_bug.cgi?id=1670656 --- Comment #6 from Xavier Bachelot <xavier@xxxxxxxxxxxx> --- Thanks Mark, the specfile definitely starts to look better. Again, more comments : - About the webpack tarball. How is it generated ? Can you add either an URL, a comment or a script ? The spec needs to make clear how to recreate it from scratch. (Later comment after looking at the git repo, you do have the script already, add it as a Source with a comment above) Also, I'm not sure to which extend such bundling is allowed or not, but I'll leave it alone for now and defer to people who know better. I know from first hand experience that unbundling may consume a lot of time, and keeping it that way is likely the best short term solution. Meanwhile, you should add versioned Provides: bundled(nodejs-blahblah) = x.y.z for each of the bundled module. Same for go modules, indeed. https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling - The -p when creating directories is useless, there's nothing to preserve. - You don't need to create %{buildroot}/%{_unitdir}, it's already there because of the BR: systemd. - The rundir needs to be handled by tmpfiles.d https://docs.fedoraproject.org/en-US/packaging-guidelines/Tmpfiles.d/ - About doc installation, you can make it simpler Remove '''install -p -m 644 docs/VERSION *.md %{buildroot}/%{_datadir}/doc/%{name}''' The 'VERSION' file is useless. 'NOTICE.md' is not a license file, move it to %doc. Modify the %files section to read: %license LICENSE.md %doc CHANGELOG.md CODE_OF_CONDUCT.md CONTRIBUTING.md NOTICE.md %doc PLUGIN_DEV.md README.md ROADMAP.md UPGRADING_DEPENDENCIES.md - Still about doc, you should build the content of docs/ and ship it, possibly in a separate sub-package if it is too big' - About %post, don't source the sysconfig file to get the grafana user, group and home. It doesn't make sense, as the sysconfig file shipped by the rpm have that set already. Anyway, the user/group creation belongs to %pre, not %post, as the user/group is needed by rpm to set ownership of the dirs and files it deploys. And when in %pre, you don't have the sysconfig file at all. Also add 'Requires(pre): shadow-utils' - %preun/%postun are missing. From https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets %systemd_post grafana-server.service ''' %preun %systemd_preun grafana-server.service %postun %systemd_postun_with_restart grafana-server.service ''' - Add a blank line between changelog entries for legibility. - Missing logrotate conf ? Or is grafana taking care of rotating its log on its own ? -- 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://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
