https://bugzilla.redhat.com/show_bug.cgi?id=1795461 --- Comment #19 from Jiri Hladky <hladky.jiri@xxxxxxxxx> --- Hi Ankur, thanks a lot for the review! My answers are below: > upstream doesn't have the 0.951 release here: https://sourceforge.net/projects/pracrand/files/ > The source URL you use is your own fork: https://github.com/jirka-h/PractRand/ > So, are we packaging your fork here? Yes, for the time being. The author is currently not responding and several forks on GitHub have been created. I have taken the last development version I got from the author. (We have collaborated to prepare the release on Linux.) I made some minor modifications, mainly regarding the documentation and man pages: https://github.com/jirka-h/PractRand/commits/master I plan to merge the fork as soon as the author will resume the development. > - Please remove the license.txt file from docs and mark it using the %license macro > License file license.txt is not marked as %license See: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text Completed. Thanks for the hint! > - You also do not need to copy the docs to the docdir. You can just use: %doc doc/ in %files and that'll copy over the files. Fixed. > - build flags aren't used in the compilation commands. Fixed. > - should the package include a -devel sub-package that includes headers and so on? I have been thinking about it but I came to the conclusion it's not a good idea for several reasons: - software is developed as an application, not as a library. Without upstream support, creating a shared library is not recommended: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_shared_libraries - API is not documented - there is an excellent command-line interface to test any data from stdin. This is a very flexible approach and it follows the *nix strategy of small utilities performing one task and performing it well. - to test custom RNG using the current implementation you have to create a C++ class derived from PractRand::RNGs class for your RNG. See also https://github.com/jirka-h/PractRand/blob/master/doc/Tests_overview.txt#L83 Let me cite :"it tends to require more work as you have to understand some of the subtleties of the internals of PractRand." - on systems with multiple CPUs, using one process to generate test data and running practrand-RNG to test it as another process, can run actually faster than integrating new RNG to the practrand-RNG test. This is because the pipe approach can use more CPUs. You are wasting some resources by sending the data over the pipe, but you are using more CPUs which outweighs the cost of sending data through the pipe. Tested with jsf64 on a laptop with 4 cores (8 CPUs thanks to hyperthreading): time ./practrand-RNG_test jsf64 -tlmax 4G -multithreaded real 0m16.555s user 0m44.767s sys 0m0.146s time { ./practrand-RNG_output jsf64 $(bc <<< 4*1024^3) | ./practrand-RNG_test stdin32 -tlmax 4G -multithreaded ; } real 0m16.065s user 0m48.656s sys 0m2.445s The latter approach is faster (comparing real time) but takes more resources (compare user time). > Any reason to not use the included Makefile? Makefile coming with the source code is currently broken. The author has plans to fix it in the new version. When it's ready, I will switch to use it. Updated SPEC file and SRPM: Spec URL: https://jhladky.fedorapeople.org/practrand.spec SRPM URL: https://jhladky.fedorapeople.org/practrand-0.951-2.fc32.src.rpm Could you please have a look? Thanks! Jirka -- 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 Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
