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

refactor: extract CCheckQueue's data handling into a separate container "Bag" #27331

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

martinus
Copy link
Contributor

CCheckQueue has stored its work items in a queue, but made no guarantee about the order of elements in that container. This PR extracts that data storage handling into a separate container class Bag. This is pure refactoring, the result should have a better separation of concerns, adds tests for the new container, and makes it now easier to separately test and optimize the container.

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 25, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK jonatack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

Copy link
Contributor

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach ACK 6b0537c

Needs trivial rebase, an adjacent Makefile entry and include header.

This introduces a simple container where elements can be added and
removed. The order of element removal is not specified and might change
in future due to e.g. optimizations.

The logic for that container is purely copied from CCheckQueue's current
implementation of the queue.
This simplfies CCheckQueue's Loop by using the new Bag container.
@martinus
Copy link
Contributor Author

Thanks @jonatack , I've rebased 6b0537c -> 6a9c6ea. Fixed Makefile & include conflict, and added a reserve() in a unit test to fix a clang-tidy warning.

@jonatack
Copy link
Contributor

ACK 6a9c6ea

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

Successfully merging this pull request may close these issues.

None yet

3 participants