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@m4x.org 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/tradem... 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... ----------------------------------------------