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@xxxxxxxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+ --- Comment #6 from Thomas Spura <tomspur@xxxxxxxxxxxxxxxxx> 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/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 :) 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review