https://bugzilla.redhat.com/show_bug.cgi?id=831878 Thomas Spura <tomspur@xxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@xxxxxxxxxxxxxxxxx |tomspur@xxxxxxxxxxxxxxxxx Flags|needinfo?(tomspur@fedorapro |fedora-review? |ject.org) | --- Comment #6 from Thomas Spura <tomspur@xxxxxxxxxxxxxxxxx> --- (In reply to comment #5) > Problem 1: > >[!]: MUST Buildroot is not present > >See https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag > I'm not sure what it is complaining about here. I use %{buildroot} in the > .spec. Further, the wiki says... "Fedora (as of F-10) does not require the > presence of the BuildRoot tag in the spec...". Some checklists still have this item in it. EPEL-5 still needs it, when you are not building for it, you can delete it again: https://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#Distribution_specific_guidelines > Problem 2: > >[!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the > > beginning of %install. > >See https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag > Huh? The spec file does exactly this that. Here are the relevant lines from > my spec: > %install > rm -rf %{buildroot}/* <--- See. > make PREFIX=%{buildroot}/ install Same here, can be omitted, but you are deleting %{buildroot}/* and not %{buildroot}, which makes a difference for .foo files: $ mkdir a $ touch a/.b $ rm a/* rm: cannot remove `a/*': No such file or directory $ ls a/.b a/.b > Problem 5: > >> ovirt-log-collector.noarch: W: manual-page-warning /usr/share/man/man8/engine-log-collector.8.gz 28: name expected (got a special character): treated as missing > > Not sure what this means, but it's a warning anyway. > Yeah, I have no idea how to fix this. $ man --warning -l ./src/rhev/engine-log-collector.8 > /dev/null <standard input>:28: name expected (got a special character): treated as missing And line 28 contains: 28 .\' Describe engine\-slimmed And this is a proper comment in the man page, which won't be renderen by man without the warning (If that was what you wanted?): 28 .\" Describe engine\-slimmed > Problem 6: > > It would be best to include this as engine-log-collector.8.* > I tried this and rpmlint complained with the following: > "ovirt-log-collector.noarch: W: manual-page-warning > /usr/share/man/man8/engine-log-collector.8.gz 28: name expected (got a > special character): treated as missing" Same warning as above, fine otherwise. > Problem 7: > >> ovirt-log-collector.noarch: E: non-readable /etc/ovirt-engine/logcollector.conf 0600L > >Could you change it to 0644 ? > Actually, 0600 is the right permission set. The user *could* choose > optionally to set the password for the oVirt RESTful API in there. Could you add a note in the spec file, so it's findable? :) * Please upload also the spec file and link to the spec/srpm on each update in the review requests. * "make PREFIX=%{buildroot} install" would be nicer as %{buildroot}/ or you'll have two slashes in the final install command. * It would be great to add a %check section as there is src/rhev/tests.py, if possible. * I don't like this: Source0: http://kojak.fedorapeople.org/ovirt-log-collector-%{version}.tar.gz Wouldn't it be better to release proper tarballs over here: http://www.ovirt.org/releases/stable/src/ ? Uploading the source at $random_website to fullfil the review doesn't work. When there aren't tarballs released, you'd need to describe how to the the sources from revision controll systems instead: https://fedoraproject.org/wiki/Packaging:SourceURL I consider the Source0 problem as a blocker, the rest above are nitpicks :) -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review