[Bug 1828205] Review Request: doctest - fast header-only C++ unit testing

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

 



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



--- Comment #13 from Nick Black <dank@xxxxxxxxx> ---
Hi Vitaly, thanks for the review!

> Are you still interested in this package? If not, I will take it for myself.

I last commented on it two days ago, and my sponsor has yet to reply. I have
not lost interest in it over the weekend, no.

> > Source2:        https://nick-black.com/dankamongmen.gpg
> 1. You cannot use your own GPG signatures. You should verify tarballs
> **only** if upstream provide valid signed tarballs. The doctest upstream
> don't do this => you must not check anything.

Removed both these lines. Removed gnupg from BuildRequires.

> > %setup -q -n doctest-%{version}
> 2. Replace by %autosetup.

I needed to do this to name the package doctest-devel. I see that in #6 you
suggest renaming it doctest. Please see comments #1--#3 where this is
discussed. Your recommendation appears to contradict the advice given me by
David Cantrell.

> > mkdir -p %{buildroot}/%{_docdir}/doctest
> 3. Remove this row and use only %doc directive.

I believe that without this line, I got an error during "make install". I'll
test with your change and see, but this was only added due to a need for it.

> 4. Also I suggest to use ninja instead of legacy makefiles:
> %prep
> %autosetup -p1
> mkdir -p %{_target_platform}
> 
> %build
> pushd %{_target_platform}
>     %cmake -G Ninja \
>     -DCMAKE_BUILD_TYPE=Release \
>     -DDOCTEST_WITH_MAIN_IN_STATIC_LIB:BOOL=OFF \
>     -DDOCTEST_WITH_TESTS:BOOL=ON \
>     ..
> popd
> %ninja_build -C %{_target_platform}

It was my understanding that Ninja is faster for *rebuilds*, not initial
builds. I'll go ahead and run with both and see if the timings are different.

> 5. Enable tests:
> 
> %check
> pushd %{_target_platform}
>     ctest --output-on-failure
> popd

Good call. Done.

> You must use arched package to verify that all tests will pass on all
> architectures.

I'm using an arched package.


-- 
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
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx




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

  Powered by Linux