-
-
Notifications
You must be signed in to change notification settings - Fork 91
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
Fix the bloom filter synopsis deserialization #1216
Conversation
97fbe7a
to
3e8f66b
Compare
3e8f66b
to
b1fd3f5
Compare
b1fd3f5
to
397c81a
Compare
a28f068
to
12b87da
Compare
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.
This looks alright to me, modulo a few wording issues, and please factor out the Nix changes into a separate PR. I did not review them.
@@ -73,7 +88,7 @@ class buffered_address_synopsis : public synopsis { | |||
params.p = p_; | |||
params.n = next_power_of_two; | |||
VAST_DEBUG_ANON("shrinked address synopsis to", params.n, "elements"); | |||
auto& type = this->type(); | |||
auto type = detail::annotate_type(this->type(), params); |
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.
Please rename to annotated_type
.
@@ -73,7 +88,7 @@ class buffered_address_synopsis : public synopsis { | |||
params.p = p_; | |||
params.n = next_power_of_two; | |||
VAST_DEBUG_ANON("shrinked address synopsis to", params.n, "elements"); |
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.
shrinked -> shrunk generally, and here to shrinks because it has yet to happen.
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.
Everywhere please. :-)
Move the `annotate_type()` helper function into a real namespace, since clang doesnt like functions in unnamed namespaces that are never called, emitting a warning that stops the build.
12b87da
to
e596221
Compare
The CI on Mac fails with a known issue: https://app.clubhouse.io/tenzir/story/20882/race-condition-in-node-queries-unit-test As discussed offline, I'm merging this now and will include the final style changes in a follow-up PR. |
📔 Description
📝 Checklist
🎯 Review Instructions