-
Notifications
You must be signed in to change notification settings - Fork 179
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/correct misleading qos overwrite warning #1806
Fix/correct misleading qos overwrite warning #1806
Conversation
PR missing one of the required labels: {'bug', 'documentation', 'new feature', 'dependencies', 'internal', 'breaking-change', 'enhancement'} |
607be05
to
087636c
Compare
PR missing one of the required labels: {'breaking-change', 'internal', 'documentation', 'bug', 'new feature', 'dependencies', 'enhancement'} |
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 address nitpick comment. Other than that it LGTM.
zenoh/src/api/builders/publisher.rs
Outdated
} | ||
break; | ||
if let Some(node) = nodes_including.first() { | ||
qos_overwrites = node.weight().unwrap().clone(); |
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 replace the unwrap
with expect("first node weight should not be None")
4ebe2fd
to
4441dab
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1806 +/- ##
==========================================
+ Coverage 70.22% 70.34% +0.11%
==========================================
Files 358 358
Lines 64584 64586 +2
==========================================
+ Hits 45357 45433 +76
+ Misses 19227 19153 -74 ☔ View full report in Codecov by Sentry. |
* Filter on actually overwriting nodes when applying QoS overwrites * Refactor and improve warning message about multiple QoS overwrites matching one publisher --------- Co-authored-by: Hugo Laloge <hugolaloge@gmail.com>
Refactor
PublisherBuilder::apply_qos_overwrites
and only shows the warning if multiple nodes with overwrites matches the publisher.Improve the warning to show all matching key expressions.
Fixes #1805