feat: simplify init command to better manage target MES version#736
feat: simplify init command to better manage target MES version#736manel27 wants to merge 1 commit intodevelopment-6xfrom
Conversation
.NET Test Results |
|
| args.AddRange(new [] {"--testScenariosNugetVersion", x.testScenariosNugetVersion}); | ||
| var configTxt = this.fileSystem.File.ReadAllText(x.config.FullName); | ||
| dynamic configJson = JsonConvert.DeserializeObject(configTxt); | ||
| x.ngxSchematicsVersion = configJson?["ngxSchematicsVersion"]?.Value; |
There was a problem hiding this comment.
This config file is the parameters file from our CustomerPortal, it does not have any mention of ngxSchematicsVersion.
I believe the goal was to determine the ngxSchematics version through its npm tag which is the MES version, like a ngxSchematics version with the tag release-1126 is the version that should be used for MES 11.2.6.
https://www.npmjs.com/package/@criticalmanufacturing/ngx-schematics?activeTab=versions
There was a problem hiding this comment.
correct, ngxSchematics is supposed to be optional and, if omitted, assumed using the dist-tag of the version. I think the dist-tag was not directly supported in the option (it was a SemanticVersion object and a dist-tag will not conform to that) but not sure if this was fixed meanwhile.
There was a problem hiding this comment.
Checking the code, when running ngx-schematics commands it seems that this is trying to be done (for example here) but before this line there is a check that throws an exception if the ngxSchematicsVersion is null, so basically it does nothing 😅 .
I think this logic should be directly in the init command and these fallbacks that seemingly do nothing should be cleaned up.
There was a problem hiding this comment.
Don't forget to validate that the tests are passing, specially the New class tests.
3 participants
What was done
In init command nugetVersion, testScenariosNuget and ngxSchematicsVersion are not required at launch.
They all use the version from MESVersion flag except the last one that also checks the config file if not mentioned in the command line.
Validation checklist
Please ensure the following conditions are met in order for this PR to be merged: