-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modernize event display example steering #178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. A few smallish comments inline.
As far as the README is concerned, I would do the following:
- Remove all the outdated parts that were describing the steps that were necessary to get from the automatically converted on to a working version related to handling of EDM4hep inputs.
- Remove the explanation of the detailed technical explanation of running things through the MarlinWrapper and rather reference other existing documentation for this
- Update the section of how this script was obtained originally (mainly rephrasing the last two bullet points to some general information about handling EDM4hep input and replacing
InitDD4hep
with theGeoSvc
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the only thing left to do would be to make the compactFile a command line argument. After that I would merge this (with the EventSelector removed).
done (I hope) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the typo, I think this is good to go.
Thanks. Failing CI is now only due to known issues. |
BEGINRELEASENOTES
ENDRELEASENOTES
I modernized the event display example a bit and made it able to deal with both lcio and edm4hep files.
The corresponding documentation still needs to be updated. However, it contains a lengthy step by step description of the steering file content that is no longer valid and the only thing I can offer regarding those parts is to delete them 🤷 but before doing so I wanted to gather some feedback :)