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



--- Comment #3 from Jan Friesse <[email protected]> ---
@Robert-André Mauchin

Thank you a lot for the review!


>  - Use set_build_flags
> 

This is neat feature, I didn't notice that it was added.

> %build
> %set_build_flags
> make \
> %if %{defined use_vmguestlib}
>     WITH_VMGUESTLIB=1 \
> %else
>     WITH_VMGUESTLIB=0 \
> %endif
>     %{?_smp_mflags}
> 
>  - Use install -p to keep timestamps:

Done

> 
> %if %{with systemd}
> mkdir -p %{buildroot}/%{_unitdir}
> install -pm 755 init/%{name}.service %{buildroot}/%{_unitdir}
> %else
> mkdir -p %{buildroot}/%{_initrddir}
> install -pm 755 init/%{name} %{buildroot}/%{_initrddir}
> %endif
> 
>  - If it's only for Fedora maybe you can drop the non-systemd version? Apart
> from RHEL6, I don't think you'll need chkconfig stuff.

Yep, agreed. This package is for Fedora only, so I've removed chkconfig stuff.

> 
>  - Ask upstream for a LICENSE file

I'm upstream :) so I've added both COPYING and AUTHORS files.

> 
>  - The version in the header and the %changelog is not the same

Good catch, fixed.

Fixed SRPM is
https://honzaf.fedorapeople.org/spausedd/spausedd-20190319-1.fc29.src.rpm, SPEC
https://honzaf.fedorapeople.org/spausedd/spausedd-20190319.spec and scratch
build https://koji.fedoraproject.org/koji/taskinfo?taskID=33628072

Thank you again for the review,
  Honza

-- 
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 -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]

Reply via email to