-
Notifications
You must be signed in to change notification settings - Fork 141
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 -Wconversion and other warnings #281
Conversation
Signed-off-by: Anton Pryakhin <[email protected]>
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.
Have some proposals and comments
Signed-off-by: Anton Pryakhin <[email protected]>
Signed-off-by: Anton Pryakhin <[email protected]>
Signed-off-by: Anton Pryakhin <[email protected]>
Signed-off-by: Anton Pryakhin <[email protected]>
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.
Almost there, the only concerning comment is about messageCount
ceil
float messageCount = ceil(k_NUM_RECORDS / 3.0); | ||
float outstandingRatio = float(partiallyConfirmedGUIDS.size() + 1) / | ||
messageCount * 100.0; | ||
const size_t messageCount = k_NUM_RECORDS / 3 + 1; |
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.
Here, if k_NUM_RECORDS == 3
:
the previous code will have messageCount == 1.0f
the updated code will have messageCount == 2
This should help
const size_t messageCount = k_NUM_RECORDS / 3 + 1; | |
const size_t messageCount = (k_NUM_RECORDS + 2) / 3; |
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.
Nice suggestion! The test won't break if we change k_NUM_RECORDS
in the future
100.0; | ||
const size_t messageCount = k_NUM_RECORDS / 3; | ||
const float outstandingRatio = float(outstandingGUIDS.size()) / | ||
float(messageCount) * 100.0f; |
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.
Might be good to use C++ static_cast
s here and below, not the C-style casts. We can do it in a separate PR if we don't want to increase the scope of this one
#else | ||
const size_t k_NUM_ELEMENTS = 1000 * 1000; // 1M | ||
const int k_NUM_ELEMENTS = 1000 * 1000; // 1M |
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.
const int k_NUM_ELEMENTS = 1000 * 1000; // 1M | |
const int k_NUM_ELEMENTS = 1000 * 1000; // 1M |
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.
Yeah, this formatting looks strange, but that's exactly what clang-format did. CI fails otherwise, check this log
Signed-off-by: Anton Pryakhin <[email protected]>
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.
Perfect!
* Fix mwc and mqb conversion warnings * Fix bmqstoragetool warnings Signed-off-by: Anton Pryakhin <[email protected]>
* Fix mwc and mqb conversion warnings * Fix bmqstoragetool warnings Signed-off-by: Anton Pryakhin <[email protected]>
* Fix mwc and mqb conversion warnings * Fix bmqstoragetool warnings Signed-off-by: Anton Pryakhin <[email protected]>
No description provided.