Refactor container handling to include Technology attribute in multip…#169
Conversation
…le writers and update sample CSV files accordingly
SlavaVedernikov
left a comment
There was a problem hiding this comment.
I don't think it's safe to change IAaCWriter
from
public IAaCWriter AddContainer(string softwareSystemName, string name, string? containerType = null, string? label = null, string? description = null);
to
public IAaCWriter AddContainer(string softwareSystemName, string name, string? containerType = null, string? technology = null, string? label = null, string? description = null);
by adding string? technology = null in the middle of arguments as all the arguments at the end are of string? data type, so if there are any uses, label can be confused for technology and description can be confused for label
Add string? technology = null to the end instead, please i.e.
public IAaCWriter AddContainer(string softwareSystemName, string name, string? containerType = null, string? label = null, string? description = null, string? technology = null);
… Technology is the last argument
This is all set with the latest commit. |
I am not able to make changes to Excel files, in my corporate environment, without also applying a Sensitivity Label. I don't have access to Excel from my home computer. Can you please make the changes to the excel files? |
Sure, I'll take care of Excel files, no problem. |
| public string Type { get; set; } | ||
|
|
||
| [Name("Alias")] | ||
| [Name("Technology")] |
There was a problem hiding this comment.
Could you add Technology field as the last field here i.e. with [Index(6)]
…in CsvDataProvider to be in last position
| InternetBanking,Mobile App,Mobile,InternetBanking.Containers.MobileApp, | ||
| InternetBanking,API Application,Spa,InternetBanking.Containers.APIApplication, | ||
| InternetBanking,Database,Database,InternetBanking.Containers.Database, | ||
| Software System,Name,Type,Technology,Alias,Description |
There was a problem hiding this comment.
Please move Technology to the end
| TraderX,Trading Services,Api,TraderX.Containers.TradingServices,Service which provides trading services | ||
| TraderX,Accounts Service,Api,TraderX.Containers.AccountsService,Service which provides account management | ||
| TraderX,People Service,Api,TraderX.Containers.PeopleService,Service which provides user details management | ||
| Software System,Name,Type,Technology,Alias,Description |
There was a problem hiding this comment.
Please move Technology to the end
There was a problem hiding this comment.
Should be all set. Apologies for not also making this change with the prior commit. I will try to be more careful in the future and take the time to test any changes with the samples. May be good to get some unit tests added to project down the road so changes can be tested in an automated way.
There was a problem hiding this comment.
No problem :)
I agree about unit tests... We have a pull request for unit tests, but it requires some work. I'll try and pick it up some time soon.
Thanks
…to be last in CSV files
Summary
Adds support for the
Technologyproperty in CSV Container definitions, bringing CSV format parity with JSON/YAML formats.Closes #168
Changes Made
1. CSV Data Model
Technologyproperty toCsvDataProvider.Containerrecord (Index 4)2. Interface Updates
IAaCWriter.AddContainer()signature to includetechnologyparameter3. Implementations Updated
Writers:
CsvToCSharpAaCWriterYamlToCsvAaCWriterCsvToJObjectAaCWriterJsonToCSharpAaCWriterCSharpToCSharpAaCWriterCSharpToYamlAaCWriterGenerators:
CsvToCSharpAaCGeneratorCsvToYamlAaCGeneratorCsvToJsonAaCGeneratorYamlToCsvAaCGeneratorJsonToCSharpAaCGenerator4. Sample Files
Updated example CSV files with technology values:
Samples/Internet Banking System/CSV/Architecture Catalogue - CSV Export/Containers.csvSamples/TraderX/CSV/Architecture Catalogue - CSV Export/Containers.csvTesting
Breaking Changes
Containers.csvfiles will need to add theTechnologycolumn in the correct position (after Type, before Alias).Migration: Add an empty
Technologycolumn or populate with technology values: