Refactor aligned printer #605
Conversation
…d of const char* Signed-off-by: Ari Raein <[email protected]>
There was a problem hiding this comment.
- Added a review notes
- CI DCO check fails for the second commit in the PR. All commits need to be DCO-signed to pass this. The instructions how it can be simply fixed are here: https://github.com/bloomberg/blazingmq/blob/main/CONTRIBUTING.md#dco-check
- CI formatter check fails, need to run clang-format on source files before commiting, or alternatively you can use CI output to make fixes: https://github.com/bloomberg/blazingmq/actions/runs/13254786590/job/37026607247?pr=605
- Build fails, might be related to review notes
| docker/sanitizers/ | ||
| lib64/ | ||
| src/groups/mqb/mqbc/mqbc_clusterutil.t.cpp | ||
| src/groups/mqb/mqbc/mqbc_clusterutil.t.cpp |
There was a problem hiding this comment.
Among these newly added items:
- Some are not expected in a normal workflow:
.bash_history
.bde_tools/
lib64/
- Some are legit project-related files or folders and there is no reason to ignore them:
docker/sanitizers/
src/groups/mqb/mqbc/mqbc_clusterutil.t.cpp
src/groups/mqb/mqbc/mqbc_clusterutil.t.cpp
Let's revert changes in this file
| @@ -1219,7 +1219,7 @@ static void test14_summaryTest() | |||
| fields.push_back("Number of partially confirmed messages"); | |||
| fields.push_back("Number of confirmed messages"); | |||
| fields.push_back("Number of outstanding messages"); | |||
| bmqu::AlignedPrinter printer(expectedStream, &fields); | |||
| bmqu::AlignedPrinter printer(expectedStream, fields); | |||
There was a problem hiding this comment.
Also need to change type a few lines above:
bsl::vector<const char*> fields -> bsl::vector<bsl::string> fields
| @@ -2108,7 +2108,7 @@ static void test24_summaryWithQueueDetailsTest() | |||
| fields.push_back("Number of partially confirmed messages"); | |||
| fields.push_back("Number of confirmed messages"); | |||
| fields.push_back("Number of outstanding messages"); | |||
| bmqu::AlignedPrinter printer(expectedStream, &fields); | |||
| bmqu::AlignedPrinter printer(expectedStream, fields); | |||
There was a problem hiding this comment.
Also need to change type a few lines above:
bsl::vector<const char*> fields -> bsl::vector<bsl::string> fields
| @@ -71,7 +71,7 @@ void printRecord(bsl::ostream& stream, | |||
| fields.push_back("GUID"); | |||
| fields.push_back("Crc32c"); | |||
|
|
|||
| bmqu::AlignedPrinter printer(stream, &fields); | |||
| bmqu::AlignedPrinter printer(stream, fields); | |||
There was a problem hiding this comment.
Also need to change type, and do the same in other places in this file:
bsl::vector<const char*> fields -> bsl::vector<bsl::string> fields
| @@ -155,7 +155,7 @@ void printRecord(bsl::ostream& stream, | |||
| } | |||
| } | |||
|
|
|||
| bmqu::AlignedPrinter printer(stream, &fields); | |||
| bmqu::AlignedPrinter printer(stream, fields); | |||
There was a problem hiding this comment.
Also need to change type above:
bsl::vector<const char*> fields -> bsl::vector<bsl::string> fields
| @@ -81,7 +81,7 @@ void printJournalFileMeta(bsl::ostream& ostream, | |||
| fields.push_back("SyncPoint DataFileOffset (DWORDS)"); | |||
| fields.push_back("SyncPoint QlistFileOffset (WORDS)"); | |||
|
|
|||
| bmqu::AlignedPrinter printer(ostream, &fields); | |||
| bmqu::AlignedPrinter printer(ostream, fields); | |||
There was a problem hiding this comment.
Also need to change type, and do the same in other places in this file:
bsl::vector<const char*> fields -> bsl::vector<bsl::string> fields
| @@ -474,7 +474,7 @@ void StorageInspector::processCommand( | |||
| fields.push_back("SyncPoint DataFileOffset (DWORDS)"); | |||
| fields.push_back("SyncPoint QlistFileOffset (WORDS)"); | |||
|
|
|||
| bmqu::AlignedPrinter printer(BALL_LOG_OUTPUT_STREAM, &fields); | |||
| bmqu::AlignedPrinter printer(BALL_LOG_OUTPUT_STREAM, fields); | |||
There was a problem hiding this comment.
Also need to change type above, and do the same in other places in this file:
bsl::vector<const char*> fields -> bsl::vector<bsl::string> fields
| it.loadAppIds(&appIdLenPairs); | ||
| it.loadAppIdHashes(&appIdHashes); |
There was a problem hiding this comment.
Also need to updates methods loadAppIds and loadAppIdHashes to use bsl::string:
And this typedef:
There was a problem hiding this comment.
Hi Evgeny. Do you agree with the below?
typedef bsl::pair<bsl::string, unsigned int> AppIdLengthPair;
void loadAppIds(bsl::vector<AppIdLengthPair>& appIds) const;
void loadAppIdHashes(bsl::vector<bsl::string>& appIdHashes) const;
Issue number of the reported bug or feature request: #
Describe your changes
A clear and concise description of the changes you have made.
Testing performed
Describe the testing you have performed to ensure that the bug has been addressed, or that the new feature works as planned.
Additional context
Add any other context about your contribution here.