Merge branch 'main' of https://github.com/8T4/c4sharp #67
Conversation
There was a problem hiding this comment.
Pull Request Overview
This appears to be a large merge commit that introduces significant new Draw.io export functionality to the C4Sharp library. The PR adds comprehensive Draw.io diagram generation capabilities alongside existing PlantUML support, including XML, SVG, and PNG export options.
Key changes include:
- Addition of complete Draw.io export functionality with multiple output formats
- Introduction of new sample projects demonstrating healthcare architecture diagrams
- Extension of existing sample projects with new diagram types
- Creation of generator utilities for JSON-driven diagram creation
Reviewed Changes
Copilot reviewed 60 out of 62 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/C4Sharp/Diagrams/Drawio/* | Core Draw.io export functionality including XML, SVG, and PNG exporters |
| src/C4Sharp.Diagrams/* | New sample healthcare diagrams demonstrating C4 architecture patterns |
| src/C4Sharp.Generator/* | JSON-to-C4 diagram generator with configurable structure definitions |
| samples/HealthcareArchitecture/* | Healthcare-specific sample project with JSON configuration support |
| samples/ModelDiagrams/* | Extended banking system samples with additional diagram types |
| generated-diagrams/* | Output files from diagram generation processes |
| static DrawioSvgExporter() | ||
| { | ||
| // This will be called once to set up SVG definitions | ||
| } |
There was a problem hiding this comment.
The static constructor is empty and serves no purpose. Consider removing it as it adds unnecessary complexity without providing any functionality.
| } |
| { | ||
| Console.WriteLine($"Exporting Draw.io diagrams to: {drawioPath}"); | ||
| new DrawioContext() | ||
| .AddDiagrams(new[] { diagramBuilder }) |
There was a problem hiding this comment.
Creating a single-element array using 'new[] { diagramBuilder }' is unnecessarily verbose. The AddDiagrams method likely accepts params, so you can pass 'diagramBuilder' directly.
| .AddDiagrams(new[] { diagramBuilder }) | |
| .AddDiagrams(diagramBuilder) |
| public static SoftwareSystem MediMatchSystem = new SoftwareSystem( | ||
| "MediMatch Middleware", | ||
| "Reconciliation engine and APIs"); | ||
| public static SoftwareSystem PharmacyDirectSystem => |
There was a problem hiding this comment.
Inconsistent indentation - this line uses 3 spaces while other lines use 4 spaces. Consider using consistent indentation throughout the file.
| public static SoftwareSystem PharmacyDirectSystem => | |
| public static SoftwareSystem PharmacyDirectSystem => |
| "Human actors verifying and approving exceptions"); | ||
| public static Person MedicalAidSchemes = new Person( | ||
| "Medical Aid Schemes", | ||
| "External providers of ERA files"); |
There was a problem hiding this comment.
Inconsistent indentation - this line uses excessive spaces (11) compared to the standard 4-space indentation used elsewhere in the file.
| "External providers of ERA files"); | |
| "Finance Team", | |
| "Human actors verifying and approving exceptions"); | |
| public static Person MedicalAidSchemes = new Person( | |
| "Medical Aid Schemes", | |
| "External providers of ERA files"); |
| ]; | ||
| } | ||
| BankingSystem > Mainframe, | ||
| }; |
There was a problem hiding this comment.
[nitpick] Inconsistent array syntax - the file mixes collection expressions '[...]' and array expressions 'new[] {...}'. Consider using consistent syntax throughout the file for better readability.
| var processInfo = new ProcessStartInfo | ||
| { | ||
| FileName = "java", | ||
| Arguments = $"-jar \"{plantumlJarPath}\" -tsvg \"{outputPath}\\*.puml\"", |
There was a problem hiding this comment.
The command line argument construction is fragile and platform-specific. The double backslashes '\' suggest Windows path handling. Consider using Path.Combine or making this cross-platform compatible.
| Arguments = $"-jar \"{plantumlJarPath}\" -tsvg \"{outputPath}\\*.puml\"", | |
| Arguments = $"-jar \"{plantumlJarPath}\" -tsvg \"{Path.Combine(outputPath, "*.puml")}\"", |
1 participant
No description provided.