[Bug 1523909] Review Request: kernel-tools - Assortment of tools for the Linux kernel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #4 from Laura Abbott <labbott@xxxxxxxxxx> ---
(In reply to Robert-André Mauchin from comment #1)

>  - Group: is not used anymore in Fedora. See:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections
> 

Done

>  - L103: generally the BuildRoot shouldn't need to be redefined.
> 

Done. I think this was a holdover from how the real kernel works.

>  - L117: cpuspeed is obsoleted but there is no corresponding Provides
> 

This has been present since 2011. I don't think it makes sense to addit now

>  - L141: python_sitearch is already defined by default, no need to redefine
> it.
> 

Done

>  - L187: No need for rm -rf $RPM_BUILD_ROOT
> 

Done

>  - Multiple times: make %{?_smp_mflags} → %make_build
> 
>  - Multiple times: make DESTDIR=%{buildroot} install → %make_install
> 

Given how there are multiple invocations of make build and make install and not
all of them can use the macro, I'd rather be consistent and just not use it at
all. It makes the spec file more unreadable. 

>  - L297-303: %clean is not needed anymore. Remove it:
> 
> ###
> ### clean
> ###
> 
> %clean
> rm -rf $RPM_BUILD_ROOT
> 

Done

>  - L308-312 Since /sbin/ldconfig is the only thing called in %post and
> %postun, use the -p syntax to avoid spawning a shell:
> 
> %post -n kernel-tools-libs -p /sbin/ldconfig
> %postun -n kernel-tools-libs -p /sbin/ldconfig
> 

Done

>  - In the %files, %defattr(-,root,root) is not needed, it is already the
> default.
> 

Done

>  - The %changelog must contain the version and release.
> 
> * Tue Nov 21 2017 Laura Abbott <labbott@xxxxxxxxxx> - VERSION-RELEASE
> 

Done

>   - Add "%license COPYING" in %files for all packages combinations (I
> believe it's in perf, python-perf, and kernel-tools-libs)

Done

Also added the systemd scriplet stuff per the instructions.

Thanks for the review. As you can tell, some of this stuff has gotten crufty. I
updated the srpm and spec file at the same location.

@Jeremy Since you have more experience here, I'll let you fix this up later. I
agree it's good cleanup to have though.

-- 
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




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux