-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implemented vanilla pack #6
Conversation
- Added `IPlcProject`, `ITwincatProject`, and `IAutomationInterface` - Added `README` with a brief description
Refactor code to use the newly added generic deserializing method for less code duplications when deserializing project files.
…oject Updated all projects to .NET 8 for consistency.
- Including `PlcProjectBelongToSolution()` and `IsTwincatProjectFileExtension()` - Resolving the appsettings.json absolution path
It used to be under `TwinGet.AutomationInterface.Test` project.
- Fixed namespace for `PackCommandValidatorTests.cs` - Added docs to `Test.Utils`
Using `Directory.EnumerateFiles()` instead of `Directory.GetFiles()`
DTO classes need not be tested.
Added more logging information during packaging.
In `TwincatUtils.cs`.
When a solution is provided, we validate whether the file exists.
In `PackageService.PackFromProjectFileAsync()`.
- In case for IPackCommand is propagated, we don't really want any side-effects from changing the its members. - Also added some functions for safe execute in `Thread` to `PackageService.cs`. For instance, `SafeExecute` and `SafeExecuteAsync`.
- By separating some logics to `ProjectFileUtils` folder/namespace
`PackageService` now uses that instead of `Thread`
Added try-catch block for `PackFromProjectFileAsync` when saving PLC project as library so we can return gracefully.
With `LogWith` method for logging a `PackagingException` given an `ILogger`
The use of ThreadLocal with `AutomationInterface` is not good.
`IsManagedLibrary` extracted to a extension method in `PlcProjectMetadataExtensions`
- Renamed methods to match the strategy pattern - Removed `DoCustomValidation` as the parameters used to initialize a strategy should have been validated
common.ps1
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to add a small comment at the start of file on what the file does? Starting to get crowded with .ps1 files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For which files do you mean? ps1 files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default it launches with only the root command.
Due to [CVE-2024-005](GHSA-68w7-72jg-6qpp).
Since Node.js 16 actions is being deprecated. See this [blog post](https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/).
There were a couple of Exceptions without the Exception suffix, decided its better to make a one comment about it rather than ping it on each file separately. |
{ | ||
#pragma warning disable CS8604 // Possible null reference argument. | ||
plcProjects.AddRange( | ||
from ProjectElement project in tcSmProject.Project.Plc.Projects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel like this could be LINQed somewhere in the future
close #5 .