Jens-G commented on PR #3524: URL: https://github.com/apache/thrift/pull/3524#issuecomment-4529523559
### Code review Found 5 issues: 1. Typo `mmdv` in test invocation — should be `--gen mmd` to match the registered generator name; `make check` will always fail with "Unable to get a generator for: mmdv" https://github.com/apache/thrift/blob/af5b0c6eaf970f4e16ec4a76875ce887191035b1/test/Makefile.am#L105-L109 2. `compiler/cpp/Makefile.am` not updated — `t_mmd_generator.cc` is added to `CMakeLists.txt` and `compiler.vcxproj` but missing from `Makefile.am`; autotools builds (`./configure && make`) will not compile the generator (every other generator including the recently-added markdown one is listed in all three build files) https://github.com/apache/thrift/blob/af5b0c6eaf970f4e16ec4a76875ce887191035b1/compiler/cpp/CMakeLists.txt#L99-L102 3. Broken `-r` behavior — `main.cc` calls `generate_program()` on every included program individually, all with `get_recursive() == true`; the `generate_program()` override then calls `emit_types_recursive()` which re-emits types from those already-processed includes, producing duplicate class definitions in each intermediate `.mmd` file; the comment "a single output file" is also incorrect (one file per program is still produced) https://github.com/apache/thrift/blob/af5b0c6eaf970f4e16ec4a76875ce887191035b1/compiler/cpp/src/thrift/generate/t_mmd_generator.cc#L93-L120 4. `emit_types_recursive` has no visited-set guard — with a diamond include (A → {B, C}, both → D), `emit_program_types(D)` is called twice, producing duplicate class definitions; Mermaid treats duplicate class names as a parse error https://github.com/apache/thrift/blob/af5b0c6eaf970f4e16ec4a76875ce887191035b1/compiler/cpp/src/thrift/generate/t_mmd_generator.cc#L113-L121 5. `"md": "Markdown"` added to `build/generate-changes.py` — this entry belongs to a separate Markdown-extension branch and appears to be an unintentional rebase artifact in this Mermaid PR https://github.com/apache/thrift/blob/af5b0c6eaf970f4e16ec4a76875ce887191035b1/build/generate-changes.py#L156-L161 🤖 Generated with [Claude Code](https://claude.ai/code) <sub>- If this code review was useful, please react with 👍. Otherwise, react with 👎.</sub> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
