Skip to content

Conversation

@LukasBreitwieser
Copy link
Contributor

@phsft-bot
Copy link

Can one of the admins verify this patch?

@Axel-Naumann
Copy link
Member

Hey Lukas - thanks a lot!
@phsft-bot build!

@pcanal
Copy link
Member

pcanal commented Aug 16, 2017

Nice. One caveat though. When running with genreflex the default is to use essentially the modifier + (request for new StreamerInfo based I/O) and there is no way (and should not) to request the old I/O implementation. So it probably does not make sense to advertise the + (which is always a no-op).

@dpiparo
Copy link
Member

dpiparo commented Aug 16, 2017

I agree with philippe. The + could be left aside as the default. For what concerns the name of the field I am not sure "modifier" is the best name possible. "StreamerOption" ? Two simple fields, which can be "NoStreamer" and "NoInputOperator"?
E.g.

 ... NoStreamer = "true" />
 ... NoInputOperator = "true" />

@dpiparo
Copy link
Member

dpiparo commented Aug 16, 2017

...also the PR should be complemented with a test in roottest.

@LukasBreitwieser
Copy link
Contributor Author

Thanks for the feedback Philippe and Danilo!
will modify it and send the corresponding tests

@martinmine
Copy link
Contributor

@phsft-bot build

@phsft-bot
Copy link

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

@LukasBreitwieser LukasBreitwieser changed the title Add support for +/-/! modifier for genreflex dictionary generation [genreflex] Add tests for noStreamer and noInputOperator xml attribute Aug 29, 2017
@LukasBreitwieser LukasBreitwieser changed the title [genreflex] Add tests for noStreamer and noInputOperator xml attribute [genreflex] Add support for noStreamer and noInputOperator xml attribute Aug 29, 2017
@dpiparo
Copy link
Member

dpiparo commented Aug 29, 2017

Hi @breitwieserCern ,
the functionality now looks good. Could you add a test in roottest about the new functionality?
This one should:

In your case the CMake section could be something like:


ROOTTEST_GENERATE_REFLEX_DICTIONARY(customStreamer customStreamer.h SELECTION customStreamer_selection.xml)

ROOTTEST_ADD_TEST(customStreamer
                  MACRO customStreamer.C
                  OUTREF customStreamer.ref
DEPENDS ${GENERATE_REFLEX_TEST})

This will generate the dictionary from customStreamer.h and customStreamer_selection.xml. The macro customStreamer.C will be then executed and you can check there that your streamer does what is supposed to do (print something? your call :) ). The output will be compared to customStreamer.ref . You can start from a simplified version of the actual classes you are working with.
Do not hesitate to contact me privately if you need more info!

Cheers,
D

@LukasBreitwieser
Copy link
Contributor Author

Corresponding tests: root-project/roottest#70

@dpiparo
Copy link
Member

dpiparo commented Aug 29, 2017

That was quick...

@LukasBreitwieser
Copy link
Contributor Author

was interrupted between sending the 2 PR ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants