[Bug 728815] Review Request: trademgen - C++ Simulated Travel Demand Generation Library

[bugzilla at redhat.com]

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 at 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/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.