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
Thomas Spura <tomspur(a)fedoraproject.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flag|fedora-review? |fedora-review+
--- Comment #6 from Thomas Spura <tomspur(a)fedoraproject.org> 2011-12-13 19:29:13 EST
---
(In reply to comment #5)
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/tra...
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 :)
Hurray ;)
> 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.
Well, telling the user he should check the config, but it's failing expectantly
looks a bit odd... But it's not a blocker, so ok.
> - 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).
Looks fine now, thanks! :)
(In reply to comment #4)
I can't find those files in boost-devel.
If they are not needed besides the tests, it should be ok, but grepping for it:
$ find | xargs grep functors.hpp
[snip]
Binary file ./trademgen/libtrademgen.so.0.2 matches
Binary file ./trademgen/libtrademgen.so matches
and
$ locate functors.hpp
/usr/include/boost/date_time/adjust_functors.hpp
[snip]
The lib really uses the boost-devel adjust_functors.hpp...
#############################################################################
APPROVED
--
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.