https://bugzilla.redhat.com/show_bug.cgi?id=1670656 --- Comment #38 from Mark Goodwin <mgoodwin@xxxxxxxxxx> --- (In reply to Xavier Bachelot from comment #37) > The diff between the very first spec in this review and what it is now is > really impressive, well done. thanks once again Xavier > > More nitpicking, you're used to it now ;-) > - Patch0 can probably be split in several pieces and part of them > upstream'ed (manpages and file perms changes at least seem relevant) OK, I've split into 3 patches (see comments in the spec) > - Still about the patch, why change PLUGINS_DIR to > /var/lib/grafana/data/plugins instead of /var/lib/grafana/plugins ? fixed. This was actually leftover from grafana v5.x, where the /var shared data was installed below /usr/share/grafana. > - For legibility, can the lua script be moved at the top of the spec and > just the results be used in the ExclusiveArch tag ? done > - You can use %{?systemd_requires} in place of the explicit > Requires(pre/post/postun): systemd done > - About > Provides: bundled(golang-github-grafana-grafana-plugin-model-devel) = 0.0.1 > if this is part of grafana, then this is not bundled. agree, that makes sense. I've removed that "Provides: bundled(..) entirely now. > - There used to be Provides: bundled(..) for all the golang stuff in the > bundle everything case (fedora <= 28 or EL <= 7). > I think it should be put back in case you want to build for EL7 (I guess > F28 doesn't really matter anymore). Those Provides would only ever be for EPEL7. I can put it back if you really want, though for EPEL we'd only ever rebase to new upstream releases - and grafana upstream updates the vendor'd golang sources whenever needed. So it seems to me the spec pollution wasn't warranted and I removed it. > - Why "chmod 644 %{SOURCE2} # silence an rpmlint non-issue" in %install ? > The file is already 644 in the srpm. Removed (no longer needed) > Last point is I'm not sure how much I like the enormous pile of bundled > nodejs stuff, but this is again an huge task and it seems to be somewhat > accepted, so... yeah, agree. And yes webpack seems to be gaining precedent, e.g. see cockpit. (BTW, has anyone ever considered a "gopack" for bundled golang sources?) Spec URL: https://mgoodwin.fedorapeople.org/grafana/grafana.spec SRPM URL: https://mgoodwin.fedorapeople.org/grafana/grafana-6.1.3-1.fc31.src.rpm koji rawhide scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=34121032 -- 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
