MINIFICPP-2712 Generate modular docs#2097
Conversation
martinzink
added
the
low-impact
There was a problem hiding this comment.
Pull request overview
This PR refactors the documentation generation code to support modular documentation in addition to the existing monolithic documentation. The refactoring moves procedural code into two classes (MonolithDocumentation and ModularDocumentation) to better separate concerns and introduce the new modular documentation feature.
Changes:
- Refactored documentation generation from procedural to class-based approach with MonolithDocumentation and ModularDocumentation classes
- Added support for generating per-module documentation files in a
modules/subdirectory - Applied code formatting improvements (brace placement, range-based for loop spacing) to comply with .clang-format settings
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
With this PR, ./minifi --docs <output_dir> will generate additional md files one per extension <output_dir>/modules Closes #2097 Signed-off-by: Martin Zink <martinzink@apache.org>
With this PR, ./minifi --docs <output_dir> will generate additional md files one per extension <output_dir>/modules Closes #2097 Signed-off-by: Martin Zink <martinzink@apache.org>
minifi_main/AgentDocs.cpp
Outdated
|
|
||
| docs << "\n\n## Table of Contents\n\n"; | ||
| for (const auto& [name, documentation] : class_descriptions) { | ||
| for (const auto& [name, documentation]: class_descriptions) { |
There was a problem hiding this comment.
Why did you remove the spaces before the colons (here and at several other places in this file)? The Google style guide recommends a space before and after the colon: https://google.github.io/styleguide/cppguide.html#Horizontal_Whitespace, and that's what we have in at least 90% of minifi code, as well. Please put them back.
There was a problem hiding this comment.
I've just auto formated the file, it seems like our clang-format didnt agree with our stated style guide
I've modified that aswell fix clang-format
minifi_main/AgentDocs.cpp
Outdated
| class MonolithDocumentation { | ||
| public: | ||
| explicit MonolithDocumentation() { | ||
| for (const auto& [bundle_id, component]: minifi::ClassDescriptionRegistry::getClassDescriptions()) { |
There was a problem hiding this comment.
The second variable has type minifi::Components. Why did you rename it from components to component? Please revert this.
There was a problem hiding this comment.
minifi_main/AgentDocs.cpp
Outdated
| class ModularDocumentation { | ||
| public: | ||
| static void write(const std::filesystem::path& docs_dir) { | ||
| for (auto& [bundle_id, component]: minifi::ClassDescriptionRegistry::getMutableClassDescriptions()) { |
There was a problem hiding this comment.
does this have to be mutable?
There was a problem hiding this comment.
if we want to sort them yeah, I agree it might not be nicest but either this or we copy them into another struct or copy them in the loop?
minifi_main/AgentDocs.cpp
Outdated
| class ModularDocumentation { | ||
| public: | ||
| static void write(const std::filesystem::path& docs_dir) { | ||
| for (auto& [bundle_id, component]: minifi::ClassDescriptionRegistry::getMutableClassDescriptions()) { |
There was a problem hiding this comment.
I would name this components, as well
There was a problem hiding this comment.
minifi_main/AgentDocs.cpp
Outdated
| std::ranges::sort(components.processors, {}, &minifi::ClassDescription::short_name_); | ||
| std::ranges::sort(components.controller_services, {}, &minifi::ClassDescription::short_name_); | ||
| std::ranges::sort(components.parameter_providers, {}, &minifi::ClassDescription::short_name_); |
There was a problem hiding this comment.
This would put PutSQL before PutSmb, for example. I don't know if we currently have two such names within one module, but we might in the future. I would prefer a case-insensitive sort, unless you have a good reason to prefer this case-sensitive one.
There was a problem hiding this comment.
With this PR, ./minifi --docs <output_dir> will generate additional md files one per extension <output_dir>/modules
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically main)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.