Skip to content

refactor: enhance m_bmqstoragetool_parameters.h - #1583

Open
Mauricio0704 wants to merge 3 commits into
bloomberg:mainfrom
Mauricio0704:refactor/1507-enhance-parameters-header
Open

refactor: enhance m_bmqstoragetool_parameters.h#1583
Mauricio0704 wants to merge 3 commits into
bloomberg:mainfrom
Mauricio0704:refactor/1507-enhance-parameters-header

Conversation

@Mauricio0704

Copy link
Copy Markdown

Closes #1507

  • Remove unused includes from the header and add direct includes to printer.h and searchresult.h
  • Move record-type and print-mode constants into an unnamed namespace in the .cpp as bsl::string_view
  • Add allocator traits to CommandLineArguments and Parameters
  • Store d_allocator_p in each class and remove allocator argument used in other functions

@678098 could you take a look when you have a moment? Thanks!

@Mauricio0704
Mauricio0704 requested a review from a team as a code owner July 1, 2026 18:40
@kaikulimu

Copy link
Copy Markdown
Collaborator

@dorjesinpo Will this conflict with your storage writer?

Comment on lines +30 to +33
// MQB
#include <mqbs_datafileiterator.h>
#include <mqbs_journalfileiterator.h>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// MQB
#include <mqbs_datafileiterator.h>
#include <mqbs_journalfileiterator.h>

These includes might be replaced with forward declarations because the types are only used as pointers in this header

Comment on lines +64 to +66
#include <mqbs_datafileiterator.h>
#include <mqbs_filestoreprotocolprinter.h>
#include <mqbs_journalfileiterator.h>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#include <mqbs_datafileiterator.h>
#include <mqbs_filestoreprotocolprinter.h>
#include <mqbs_journalfileiterator.h>
#include <mqbs_filestoreprotocolprinter.h>

These 2 includes can also be replaced with forward declarations to reduce the burden of including too much

@Mauricio0704

Copy link
Copy Markdown
Author

Thanks for the feedback. I've pushed the changes. The MQB iterators are now forward-declared in the headers, and the includes moved to the .cpp files that dereference them.

@678098

678098 commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Hi @Mauricio0704, the UI shows unresolved merge conflicts. Could you rebase your branch to the latest main and solve them?

- Remove unused includes from the header and add direct includes to printer.h and searchresult.h
- Move record-type and print-mode constants into an unnamed namespace in the .cpp as bsl::string_view
- Add allocator traits to CommandLineArguments and Parameters
- Store d_allocator_p in each class and remove allocator argument used in other functions

Signed-off-by: Mauricio0704 <mausancarr0704@gmail.com>
Signed-off-by: Mauricio0704 <mausancarr0704@gmail.com>
@Mauricio0704
Mauricio0704 force-pushed the refactor/1507-enhance-parameters-header branch from 8cc2f7f to 673b291 Compare July 27, 2026 17:07
@Mauricio0704

Copy link
Copy Markdown
Author

Hi @678098, I resolved the conflict by dropping the JSON-mode validation that #1556 removed. The PR shows mergeable now. Thanks!

Comment thread src/applications/bmqstoragetool/m_bmqstoragetool_searchresult.h
Signed-off-by: Mauricio0704 <mausancarr0704@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: enhance m_bmqstoragetool_parameters.h

3 participants