-
Notifications
You must be signed in to change notification settings - Fork 21
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
Update G4DarkBreM with configurable A' scaling and decay #1386
Conversation
Thank you for linking the PR into G4DarkBreM, I am going to mark this as a draft until we can have this PR be updating G4DarkBreM to a new release version. (It is still helpful to have this PR open however since then we can still make sure G4DarkBreM's updates are compatible with ldmx-sw.) |
Alright :) with the G4DarkBreM updates merged and a new version released, let's update the submodule reference here to be the tagged release version and then we can re-run the tests and do a final review prior to merge. The only test that failed when you first opened the PR was the C++ formatting test, so I've included the instructions on how to format your C++ code as well. Set G4DarkBreM to v2.1.0
C++ FormatYou only changed one C++ file and if you were to look at the logs of the format check you could probably figure out what to change in your source file to align with the C++ Google style we use, but there is a tool in the container that does this for you.
|
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.
All the checks passed and things look good from my point of view!
@horohoo I requested your review as well to double check that this includes the updates necessary to do the studies you would like to do. I'm happy to merge, just want to make sure we didn't forget anything obvious.
@RobMina After you get a second approval, you can click the "Rebase & Merge" button and then your updates will be included in trunk
!
I don't have write access to this repo, so someone else will need to push the merge button to complete this PR. |
To be merged after LDMX-Software/G4DarkBreM#33 (which contains the updates to the G4DarkBreM submodule).