Feature/howard spillquality for develop based on j zennamo branch #407
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…he spill quality for the spill that goes with a given event, and code supporting that.
…wants to use this spill. The other checks not just for the first event but the first event OF A GIVEN STREAM for the numi case.
…add another). Thanks Gianluca
brucehoward-physics
requested review from
PetrilloAtWork,
jzennamo and
miquelnebot
jzennamo
approved these changes
Jan 17, 2024
Contributor
jzennamo
left a comment
jzennamo
left a comment
There was a problem hiding this comment.
Looked good to me and contained some things we should adopt in the BNB accounting!
miquelnebot
approved these changes
Jan 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a copy of the commits in PR 390: #390
Note from the comments/discussion in that thread that the PR there is against production patch and not develop, which is in part because there were commits not in develop that it was picking up on when telling the differences between the branch to be merged and the base branch. Looking into it, I think some of those are actually important to the module that was updated by my PR. That makes switching the linked PR back to develop (the initial suggestion) seem less favorable. While develop doesn't yet have some of the changes I noticed (like TriggerV2 -> TriggerV3), I talked to Joseph who pointed me to the PR for this branch.
Plus, as discussed with some others at the ICARUS workshop in December, we will want to have a production patch at some point (for NuMI analyses) that would incorporate this, so having a PR against that can still be useful when we go to do that.
So, here is hopefully a solution -- Ieft the old PR linked above active as it was, and I checked out Joseph's feature branch, cherry picked my 5 commits from the other PR, pushed and made this PR... Marking the same reviewer as linked commit, as well as Miquel as rel mgr and Joseph as the person whose PR/branch I'm piggy-backing on.
Is there something we can/should do to test this? And/or hopefully you will have suggestions how to proceed. Thanks!