Skip to content
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

Added SpannedAggregator to common crate #243

Conversation

Modularius
Copy link
Contributor

Summary of changes

Introduced SpannedAggregator trait to encapsulate the span-aggregating behaviour of digitiser_aggregator::Frame and nexus_writer::Run.

Instruction for review/testing

Ensure changes are as described.

@Modularius Modularius changed the title Added SpannedAggregator Added SpannedAggregator to common crate Aug 23, 2024
@DanNixon
Copy link
Member

On it's own, a trait with no uses or implementations is not really an atomic PR. What will it be used for? Is there a reason there are no implementations?

@Modularius
Copy link
Contributor Author

I wasn't sure how you wanted me to split the big PR up. This will be implemented in nexus writer and aggregator in a future PR. I thought it best to implement them separately to cut the number of "files changed" down. I had thought it might be difficult to piece apart the implementation from the other changes, but in retrospect, it makes more sense to put the basic implementations in here.

@Modularius Modularius requested a review from DanNixon August 29, 2024 14:48
@Modularius
Copy link
Contributor Author

Note some changes had to be made to the FindSpan and FindSpanMut traits to get the same functionality, but those traits will disappear in a future PR so the extra complexity will disappear.

@DanNixon DanNixon merged commit 20984f8 into STFC-ICD-Research-and-Design:main Sep 2, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants