Remediate review findings and enhance ROADMAP documentation#19
Merged
Conversation
- GRP-01: Updated verification introduction for C++ scope; added ApiMarkCpp, IContext, PathHelpers, clang OTS, and all companion artifact paths - GRP-02: Fixed ReqStream decomposition chains; added missing children to api-mark-core, api-mark-cpp, api-mark-dot-net, and api-mark-msbuild YAML - GRP-03: Wrote 8 missing requirements for C++ options, MSBuild C++ properties, and Tool CLI context properties - GRP-04: Corrected PathHelpers description (path-safety utilities) - GRP-05: Added IContext parameter to unit designs; added IMarkdownWriterFactory and IContext as External Interfaces in api-mark-dot-net design - GRP-06: Fixed --includes acceptance criterion (repeated flags, not comma-split) - GRP-07: Created clang-ast-parser.md unit design; fixed constructor vs Generate preconditions in cpp-generator.md; added ClangAstParser to introduction tree - GRP-08: Added 6 missing C++ properties to MSBuild task contract and CLI signature - GRP-09: Added 8 test scenarios for gitignore patterns, intra-doc links, and external types to cpp-generator verification - GRP-10: Added ApiHeaders criterion, WithDotNetProject scenario, and ApiHeaders_ForwardedAsIndividualFlags scenario to MSBuild verification - GRP-11: Added 5 missing C++ properties to Tool Context design and CLI reference - GRP-12: Added IContext scenario to core verification; added DoesNotThrow scenarios to i-markdown-writer; added HasCreateMarkdown scenario to factory - GRP-13: Fixed critical YAML bug (ForwardApiHeaders missing - id: prefix); added 4 missing children to api-mark-msbuild system requirements; simplified compound requirement titles - GRP-14: Added XmlDocReader, FileMarkdownWriterFactory, DemaConsulting.TestResults to introduction; fixed ToolTask naming error; removed unverified precondition; added Preconditions to 5 writer methods; added CppGenerator as Caller in core docs; fixed grammar; moved ExternalTypeInfo to Data Model - GRP-15: Created CONTRIBUTING.md; added C++ MSBuild section to installation guide; added APIMARK_CLANG_PATH env var to prerequisites; fixed References section Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use 'python -c ...' one-liner (no native DLLs, no helper script), consistent with the clang -ast-dump=json pattern for C++. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Option B uses python + hdlConvertor subprocess (consistent with clang/ast patterns) and natively preserves Doxygen comments on ports and generics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Separate symbol extraction (IApiGenerator -> ApiLibrary tree) from document rendering (IDocumentGenerator) so output formats can vary independently of language parsers. Notes multi-language library potential. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The decoupling point is format strategy (file layout) not symbol abstraction. Language generators own all symbol knowledge; format strategies own layout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Phase 1 (parsing) runs once; multiple format strategies each write to their own output directory at negligible cost. ApiMarkOutput item group for multi-format builds; scalar ApiMarkOutputDir stays backward-compatible. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sign Symbol tree populated at broadest visibility; each emitter filters independently per audience. Visibility becomes a rendering concern, not a parse-time gate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ase 2 VHDL, Phase 3 Python) Configurable Output Formats moves first as it requires refactoring DotNet and Cpp simultaneously — two emitters against two languages proves the two-stage architecture. VHDL and Python follow as greenfield implementations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request updates project documentation to remediate prior review findings while also improving C++ MSBuild integration by auto-harvesting include paths and refactoring generator internals for clearer, lower-parameter helper flows.
Changes:
- Added/expanded project documentation (ROADMAP, contributing guide, verification/design/reqstream alignment updates).
- Enhanced MSBuild C++ support by defaulting
ApiMarkIncludePathsfromClCompileAdditionalIncludeDirectorieswhen not explicitly set, and updated fixtures accordingly. - Refactored .NET and C++ generators (and related tests) to reduce parameter threading and improve internal structure; minor performance/clarity tweaks (e.g.,
FlattenArity,TryGetValuein tests).
Reviewed changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/ApiMark.MSBuild.Tests/FixturePaths.cs | Switch to Path.Join in fixture path helper. |
| test/ApiMark.MSBuild.PackageTests/PackageIntegrationTests.cs | Use Path.Join for vswhere path construction in test helper. |
| test/ApiMark.MSBuild.PackageTests/Fixtures/Cpp/SampleLib/stub.cpp | Adds a stub TU so the fixture project has ClCompile items for include-dir harvesting. |
| test/ApiMark.MSBuild.PackageTests/Fixtures/Cpp/SampleLib/SampleLib.vcxproj | Moves include configuration to AdditionalIncludeDirectories + adds ClCompile item to support auto-discovery scenario. |
| test/ApiMark.DotNet.Tests/DotNetGeneratorTests.cs | Test robustness/clarity updates (TryGetValue, shared header arrays, minor assertion tweaks). |
| test/ApiMark.DotNet.Fixtures/SampleClass.cs | Expands empty fixture method into block body with explanatory comment. |
| test/ApiMark.DotNet.Fixtures/IntVsIntArrayClass.cs | Expands empty fixture methods into block bodies with explanatory comments. |
| test/ApiMark.Cpp.Tests/FixturePaths.cs | Switch to Path.Join in fixture path helper. |
| test/ApiMark.Cpp.Tests/CppGeneratorTests.cs | Test robustness updates (TryGetValue, avoid double dictionary lookups). |
| src/ApiMark.MSBuild/build/DemaConsulting.ApiMark.MSBuild.targets | Auto-populates ApiMarkIncludePaths for .vcxproj from AdditionalIncludeDirectories when unset. |
| src/ApiMark.MSBuild/ApiMarkTask.cs | Refactors CLI arg building into helpers; improves C++ skip message. |
| src/ApiMark.DotNet/TypeNameSimplifier.cs | Uses span-based concat to reduce allocations in FlattenArity. |
| src/ApiMark.DotNet/DotNetGenerator.cs | Refactors writing pipeline with bundled context records + extracted helpers. |
| src/ApiMark.Cpp/CppTypeLinkResolver.cs | Minor simplification (EndsWith char overload). |
| src/ApiMark.Cpp/CppGenerator.cs | Refactors header pattern compilation/matching and type-page writing via context records; reduces duplication. |
| src/ApiMark.Cpp/CppAst/ClangAstParser.cs | Refactors parsing helpers, path joining, and base/nested/type-alias handling for readability. |
| ROADMAP.md | Adds roadmap describing phased future work and output-format strategy. |
| review-findings.md | Adds consolidated review findings document. |
| README.md | Updates C++ MSBuild guidance to reflect include auto-discovery. |
| grouped-issues.md | Adds grouped remediation tracking document. |
| docs/verification/introduction.md | Updates scope/artifact structure to include C++/clang/IContext/PathHelpers and removes stale out-of-scope text. |
| docs/verification/api-mark-tool/cli/context.md | Aligns --includes acceptance criteria with repeated-flag behavior. |
| docs/verification/api-mark-msbuild/api-mark-task.md | Adds missing acceptance/scenario documentation (api-headers forwarding, dotnet scenario, include auto-population scenario). |
| docs/verification/api-mark-msbuild.md | Adds dotnet scenario + aligns “cpp scenario not yet implemented” note. |
| docs/verification/api-mark-cpp/cpp-generator.md | Adds missing acceptance criteria + scenarios for patterns/links/external types. |
| docs/verification/api-mark-core/i-markdown-writer.md | Adds missing scenarios for interface “does not throw” tests. |
| docs/verification/api-mark-core/i-markdown-writer-factory.md | Adds missing scenario for CreateMarkdown contract test. |
| docs/verification/api-mark-core.md | Adds IContext acceptance criteria + scenario. |
| docs/user_guide/msbuild-integration.md | Updates ApiMarkIncludePaths docs to “auto-detected” and shows optional override. |
| docs/user_guide/introduction.md | Fixes References section (now “None.”). |
| docs/user_guide/installation.md | Adds APIMARK_CLANG_PATH note + C++ MSBuild installation section. |
| docs/reqstream/api-mark-tool/cli/context.yaml | Adds missing C++ context option requirements and test links. |
| docs/reqstream/api-mark-tool/cli.yaml | Restores missing children links for includes/api-headers/cpp named options. |
| docs/reqstream/api-mark-msbuild/api-mark-task.yaml | Adds missing MSBuild requirements for implemented behavior + test links. |
| docs/reqstream/api-mark-msbuild.yaml | Adds missing children links for new MSBuild requirements. |
| docs/reqstream/api-mark-dot-net/dot-net-generator.yaml | Clarifies titles for compound requirements (wording only). |
| docs/reqstream/api-mark-dot-net.yaml | Restores missing children links (ReqStream traceability). |
| docs/reqstream/api-mark-cpp/cpp-generator.yaml | Clarifies requirement titles/justification wording. |
| docs/reqstream/api-mark-cpp.yaml | Restores missing children links (ReqStream traceability). |
| docs/reqstream/api-mark-core.yaml | Restores missing children links (ReqStream traceability). |
| docs/design/introduction.md | Adds missing units/OTS entries (e.g., ClangAstParser, XmlDocReader, FileMarkdownWriterFactory, DemaConsulting.TestResults). |
| docs/design/api-mark-tool/cli/context.md | Adds missing C++-specific context properties to data model. |
| docs/design/api-mark-tool/cli.md | Updates Context property list + recognized options list. |
| docs/design/api-mark-msbuild/api-mark-task.md | Documents include auto-population; fixes dependency wording; removes unsupported “writability” precondition. |
| docs/design/api-mark-msbuild.md | Completes documented MSBuild C++ contract + data flow for include harvesting. |
| docs/design/api-mark-dot-net/dot-net-generator.md | Moves ExternalTypeInfo into Data Model and documents IContext parameter. |
| docs/design/api-mark-dot-net.md | Adds IMarkdownWriterFactory and IContext as consumed interfaces. |
| docs/design/api-mark-cpp/cpp-generator.md | Fixes grammar + documents IContext parameter and Generate-time include-root existence precondition. |
| docs/design/api-mark-cpp/clang-ast-parser.md | Adds new design doc for ClangAstParser utility. |
| docs/design/api-mark-core/i-markdown-writer.md | Adds missing Preconditions entries + adds CppGenerator to Callers. |
| docs/design/api-mark-core/i-markdown-writer-factory.md | Adds CppGenerator to Callers. |
| docs/design/api-mark-core.md | Corrects PathHelpers security invariant description. |
| CONTRIBUTING.md | Adds contributing guide (setup, workflow, style, bug reporting). |
| .reviewmark.yaml | Expands review coverage to .targets and the new clang parser design doc. |
| .cspell.yaml | Adds new technical terms to spelling dictionary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Note that C++ MSBuild integration works for conventional project layouts; steer complex projects toward the CLI for full control over clang arguments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Covers common questions across C#, C++, MSBuild, and CI — including how to handle complex C++ multi-target projects with a dedicated documentation step, and how to run self-validation in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MSBuild: ApiMarkTask_Cpp_LibraryDescription_ForwardedToTool, ApiMarkTask_Cpp_ClangPath_ForwardedToTool, ApiMarkTask_DotNet_EmptyXmlDocPath_SkipsExecution Tool: Context_Create_WithLibraryNameOption_SetsLibraryName, Context_Create_WithLibraryDescriptionOption_SetsLibraryDescription, Context_Create_WithDefinesOption_SetsDefines, Context_Create_WithCppStandardOption_SetsCppStandard Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Jun 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several important documentation updates and configuration improvements across the project. The most significant changes include the addition of a comprehensive contributing guide, the introduction of a project roadmap outlining future phases, and expanded or clarified documentation for C++ and core Markdown writer interfaces. There are also minor configuration updates to spelling and review settings.
Documentation Additions and Improvements:
CONTRIBUTING.mdfile with detailed guidelines for development setup, code style, submitting changes, and reporting bugs.ROADMAP.mdfile describing planned features and implementation phases, including output format strategies and support for VHDL and Python documentation generation.docs/design/api-mark-cpp/clang-ast-parser.mddetailing the purpose, data model, key methods, and external interfaces for the internalClangAstParserutility used by the C++ generator.docs/design/api-mark-cpp/cpp-generator.mdand core Markdown writer design docs to clarify preconditions, parameters, and usage for C++ and Markdown writer interfaces. [1] [2] [3] [4] [5] [6] [7]Configuration and Review Settings:
.reviewmark.yamlto add C++ and MSBuild targets files to review coverage, and included the new clang AST parser design doc for review. [1] [2] [3].cspell.yamlby adding new technical terms. [1] [2]Documentation Corrections and Clarifications:
README.mdto clarify that include paths for C++ projects are auto-discovered, removing outdated manual configuration instructions.These changes improve project maintainability, contributor onboarding, and the clarity of both internal and external documentation.