-
Notifications
You must be signed in to change notification settings - Fork 14
Smash the UniversalScanner project as a library and desktop application. #3
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
base: master
Are you sure you want to change the base?
Conversation
julienblitte
left a comment
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.
Thanks for you interest and your pull request.
I am afraid, I am not planning for now to have gRPC service for a such small tool.
Also, I would like to include Packages or change dotNet version (for compatibility issues on other tools) and rename source file (as we will lose file tracking in github).
| <configuration> | ||
| <startup> | ||
| <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.5.2" /> | ||
| <supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.7.2"/> |
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.
Please do not change framework version, this has been setup to be compliant with other software component with same version (which does not appear publicly)
| <RootNamespace>UniversalScanner</RootNamespace> | ||
| <AssemblyName>UniversalScanner</AssemblyName> | ||
| <TargetFrameworkVersion>v4.5.2</TargetFrameworkVersion> | ||
| <TargetFrameworkVersion>v4.7.2</TargetFrameworkVersion> |
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.
Please do not change framework version, this has been setup to be compliant with other software component with same version (which does not appear publicly)
| .vs | ||
| bin | ||
| obj | ||
| ## Ignore Visual Studio temporary files, build results, and |
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.
Is this .gitignore auto-generated?
Do you really need to ignore all this files? Is it because of your environment?
| </Reference> | ||
| <Reference Include="System" /> | ||
| <Reference Include="System.Core" /> | ||
| <Reference Include="System.Security.AccessControl, Version=4.1.3.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL"> |
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.
I would like to avoid extra packages or .dll file, please kindly remove this inclusion if not necessary.
I necessary, please kindly expand why.
| <HintPath>..\packages\System.Security.AccessControl.4.7.0\lib\net461\System.Security.AccessControl.dll</HintPath> | ||
| </Reference> | ||
| <Reference Include="System.Security.Principal.Windows, Version=4.1.3.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL"> | ||
| <HintPath>..\packages\System.Security.Principal.Windows.4.7.0\lib\net461\System.Security.Principal.Windows.dll</HintPath> |
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.
I would like to avoid extra packages or .dll file, please kindly remove this inclusion if not necessary.
I necessary, please kindly expand why. Is this because of your environment?
| @@ -0,0 +1,12 @@ | |||
| { | |||
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.
I would like to have one single running file at launch, as it is a very small and easy tool designed to scan network for device on customer site.
| @@ -0,0 +1,28 @@ | |||
| name: .NET Core | |||
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.
I am not using Workflow at this point, I do not think this is relevant to have it for such small project.
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "scannerTemplate", "scannerTemplate\scannerTemplate.csproj", "{CA2EBD82-FF40-4A5B-AEC6-D95104D159EF}" | ||
| EndProject | ||
| Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "scannerTemplate", "scannerTemplate\scannerTemplate.csproj", "{32E255E4-8C44-470F-911A-E29729DCFA8E}" | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "UniversalScannerService", "UniversalScannerService\UniversalScannerService.csproj", "{E69C2B74-EDC5-4362-9D7D-AA88C93C24D5}" |
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.
Sorry, but I do not see added value to split interface and core for such small tool for now.
| @@ -0,0 +1,6 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <packages> | |||
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.
Please remove packages, I tend to avoid packages to reduce use of external library.
Also, please note the version of dotNet I use is different.
| @@ -0,0 +1,39 @@ | |||
| using System; | |||
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.
Could you please kindly explain what is the added value to use Grpc on a small tool like this? I am afraid I do not see a real added value here.
It is always a good choice to separate the application into layers. The best method to develop modularly would be to separate the common layer and the application layer.