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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove caf::skip usages #2530

Merged
merged 7 commits into from Sep 2, 2022
Merged

Remove caf::skip usages #2530

merged 7 commits into from Sep 2, 2022

Conversation

patszt
Copy link
Contributor

@patszt patszt commented Aug 25, 2022 •
edited by lava

Caf::skip is deprecated in CAF 18.6
Since we are currently doing a bump to 18.6 it makes sense to push these changes separately

馃摑 Reviewer Checklist

Review this pull request by ensuring the following items:

  • All user-facing changes have changelog entries
  • User-facing changes are reflected on vast.io

dominiklohmann
dominiklohmann reviewed Aug 25, 2022
View reviewed changes
Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

I paired on this with @patszt earlier, so instead of reviewing I left some comments that should make the reviewer's job easier.

libvast/native-plugins/rebuild.cpp Show resolved Hide resolved
libvast/src/system/passive_partition.cpp Show resolved Hide resolved
libvast/src/system/passive_partition.cpp Outdated Show resolved Hide resolved
libvast/test/system/partition.cpp Outdated Show resolved Hide resolved
@dominiklohmann dominiklohmann added refactoring Restructuring of existing code bug Incorrect behavior blocked Blocked by an (external) issue labels Aug 25, 2022
@patszt patszt marked this pull request as ready for review September 1, 2022 13:32
@patszt patszt requested a review from lava September 1, 2022 13:54
@lava lava removed the blocked Blocked by an (external) issue label Sep 1, 2022
patszt and others added 3 commits September 2, 2022 11:19
@patszt
Remove caf::skip usages
86e97c2
@dominiklohmann @patszt
Shut down the REBUILDER on exit
07d994d 
Adding a custome exit handler requires calling `self->quit(...)` manually, and
we forgot to do so for the REBUILDER actor when moving away from `caf::skip`,
which requried adding a custom exit handler to clean up stored response
promises.
@dominiklohmann @patszt
Delay queries before assigning them an ID
6469db2
lava
lava approved these changes Sep 2, 2022
View reviewed changes
Copy link
Member

@lava lava left a comment

Choose a reason for hiding this comment

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

Looks very good, thanks 馃殺

libvast/src/system/passive_partition.cpp Outdated Show resolved Hide resolved
@lava lava enabled auto-merge September 2, 2022 12:46
@lava lava merged commit fadc9cc into master Sep 2, 2022
@lava lava deleted the story/sc-36803 branch September 2, 2022 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tobim tobim tobim left review comments

@dominiklohmann dominiklohmann dominiklohmann left review comments

@lava lava lava approved these changes

Assignees
No one assigned
Labels
bug Incorrect behavior refactoring Restructuring of existing code
Projects
None yet
Milestone
No milestone
4 participants
@patszt @lava @tobim @dominiklohmann