[Bug 1244797] Review Request: libASL - Advanced Simulation Library

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

 



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



--- Comment #14 from Raphael Groner <projects.rg@xxxxxxxx> ---
A quick look into your spec file shows some hints and advices. Later, I could
do the official review run. Currently, I do not find any blockers, so I hope
the review can pass quickly.

. (SHOULD/<rant>) Please use %{upstream} consistently or not at all. Well, it
seems to me somehow to be ridiculous to use a macro with a longer name than the
shorter text by itself, therefore you maybe want to replace it in %description
as well?
- Name: libASL
+ Name: lib%{upstream}

. (SHOULD) Again, please use consistently the macros. Either decide to use
"mkdir -p", %{__mkdir_p}, or "%{__mkdir} -p". I guess you want to keep with
%{__mkdir_p} as you do in %install.
  %prep
  %setup -q -n %{upstream}-%{version}
- %{__mkdir} -p %{_cmake_build_subdir}
+ %{__mkdir_p} %{_cmake_build_subdir}

. (SHOULD) examples could be merged into doc subpackage to avoid just another
subpackage without any additional benefit but that's somehow related to any
personal preference.

. (SHOULD) pushd/popd is not needed in %install
- pushd %{_cmake_build_subdir}
- %make_install
- popd
+ %make_install -C %{_cmake_build_subdir}

. (SHOULD) You could use delete parameter of find command, this makes it easier
to read and execution may be faster.
- find %{buildroot} -name '*.la' -exec rm -f {} ';'
+ find %{buildroot} -name '*.la' -delete

. (SHOULD) Please consider to use %check for execution of some files from the
tests folder. You mentioned somehow OpenCL as a possible fail reason, can you
disable individually those dedicated tests to have at least a basic QA inside
the build?

-- 
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
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]