Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=728815 --- Comment #5 from Denis Arnaud <denis.arnaud_fedora@xxxxxxx> 2011-12-12 20:53:54 EST --- Thanks for the review (I am conscious that it is not completed yet)! (In reply to comment #4) > REVIEW: > - your favorite, (ignorable in your case) rpmlint error: ;) > trademgen.src: E: invalid-spec-name Indeed, you did convince me to manage versionning of the specification file with Git in GitHub: https://github.com/denisarnaud/fedorareviews/blob/trunk/sim/trademgen/trademgen_728815/trademgen.spec In contrast, on the fedorapeople.org site, as we can not know the version of a specification without downloading it, I used to name the specification after the version, so that there is no ambiguity. For the next time, I'll use only GitHub for the specification file :) > NEEDSWORK: > - The python lib doesn't seem to work or pytrademgen has a UserError Well, that Python script is just a proof-of-concept/sample, in order to demonstrate how to expose a Python API, wrapped around the original C++ library API. It does nothing very useful for now. I'll rework it later on, so that the output seems alike what is produced by the pure C++ binary. In the meantime, I believe it is still interesting to see how to cleanly wrap the C++ library within a Python script, thanks to Boost.Python. > - license looks weired > - It looks like Boost is bundled for running the tests: > ./test/math/boost_math/empirical_distribution/count.hpp: BSL (v1.0) In fact, the test/math/ sub-directory is just a collection of samples from third party libraries. Indeed, I have benchmarked a few features of TraDemGen against some of those third party examples (that's why I left them there in the first place). For instance, a comparison of the different random generation methods/technologies can be made. However, those third party libraries are certainly not used by TraDemGen, neither by the core library, nor by the unit tests. I have therefore just transfered them into another Git repository, more appropriate for them (https://github.com/denisarnaud/playground). The test-related source tree has been cleaned (https://github.com/airsim/trademgen/tree/trunk/test). I have still to update the specification file to reflect that. Following is the updated package: ---------------------------------------------- Spec URL: http://denisarnaud.fedorapeople.org/sim/trademgen/trademgen.spec SRPM URL: http://denisarnaud.fedorapeople.org/sim/trademgen/trademgen-0.2.2-1.fc16.src.rpm ---------------------------------------------- -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review