Re: systeminfo - review package

[Denis Fateyev <denis@xxxxxxxxxxx>]

Hello,

BuildRoot is only needed for EPEL5: https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

If presents, it should be one of those: http://fedoraproject.org/wiki/EPEL:Packaging#BuildRoot_tag

Removing BuildRoot and "clean" section are needed only for EPEL5 so I think it would better to preserve conditionals.

Also, the conditionals there can be simplified: http://fedoraproject.org/wiki/Packaging:DistTag#Conditionals

%{?el5:BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root}
...
%if 0%{?el5}
%clean
rm -rf %{buildroot}
%endif

-- as examples.

---

wbr, Denis.

On Wed, Feb 19, 2014 at 5:13 AM, David Roheim <david.roheim@xxxxxxxxx> wrote:

Hi Pavol,

I can not approve your package, but I would suggest some things.

* Bump your rev version. Incl. changelog.
* Define your BuildRoot
* Use %{_bindir} instead of /usr/bin

* Source-format your spec-file

Why do you have:

%if 0%{?rhel} && 0%{?rhel} <= 5
        rm -rf %{buildroot}
%endif

Why not just:
rm -rf $RPM_BUILD_ROOT

Nice page: https://fedoraproject.org/wiki/Packaging:RPMMacros

Anyways :)

David

David Roheim

david.roheim@xxxxxxxxx - http://roheim.org/

On Tue, Feb 18, 2014 at 9:41 PM, Pavol Ipoth <pavol.ipoth@xxxxxxxxx> wrote:

Hi,

can please somebody approve my package, it was already reviewed but reviewer doesn't have permissions to approve new package. Can somebody look at it. Thanks.

https://bugzilla.redhat.com/show_bug.cgi?id=1058038

-- 
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/devel
Fedora Code of Conduct: http://fedoraproject.org/code-of-conduct