Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=947071 --- Comment #3 from Christopher Meng <cickumqt@xxxxxxxxx> --- (In reply to comment #1) > According to the files, the license is GPLv2+. Fixed. > > This seems to be a noarch package and should be labeled accordingly. > Added. > Use the name macro for your other sources as well. > I've changed all "monitorix" strings to %{name},correct? > You must BR systemd, according to > http://fedoraproject.org/wiki/Packaging:Systemd#Filesystem_locations > Fixed. > Take a look at the Perl guidelines on how and if to require Perl modules > manually. > Still have question with: perl-libwww-perl Should I changed it to "perl(LWP)" or keep current status? > I think you should create a monitorix system user. The logrotate file > doesn't match the actual file and directory layout. It should also have a su > line. Your package should either own %{_sysconfdir}/logrotate.d/ or require > logrotate > Upstream said: "I don't know if we really need the 'monitorix' user, if so it will be only used for the built-in HTTP server, which is currently using the 'nobody' user by default. For the rest of task, Monitorix needs the 'root' user in order to have enough permissions to collect data. " Well, I forgot to bring a logrotate file written by myself into package as source2... If there still has problems, please tell me. > "rm -rf %{buildroot}" in the install section should be dropped, as it is no > longer necessary. Fixed. > %{_localstatedir}/lib/monitorix/reports/*.html -- You're not owning this > directory. I also wonder why html should be configuration. Removed. > Replace the ".gz" from the manpages with an asterisk, to allow for the > compression method to change. Replaced. > What's the point of 777 permissions for that one directory? Upstream: "The only directory that needs 777 permissions is /var/share/monitorix/imgs/. This is because the user in which will run the web server will have to write there to create the graphs (.png files). Since we cannot know which web server will be used to see the graphs (the built-in HTTP server that comes with Monitorix, or Apache, or Nginx, or Lighttpd, etc.) then we cannot know which user will try to write inside the directory. That's the reason why I used 777 for that directory. " > Your changelog entry has an invalid form. See > http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs I don't know the current one is ok or not. Please have a look again. I remember a tool help generate changelog but I think copy paste is just ok with no syntax changes. > As a sidenote, you could shorten your install section by using the -D option > of the install command. You must also use -p to preserve the timestamps. Used.I tried as possible as can to shorten my install section, if there still has shortenable space, please tell me. Thanks again! -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=ZCQIbbBjc4&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review