-
Notifications
You must be signed in to change notification settings - Fork 83
[MIX][FEAT] Suggested posix compatibility changes #197
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: main
Are you sure you want to change the base?
Conversation
nimrof
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.
Dont understand the need for the .sh files
Is that for development or normal use?
The nuget updates are very welcome
|
|
||
| <ItemGroup> | ||
| <Using Include="Xunit" /> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.13.0" /> |
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.
not sure why this is moved so please move back so every package is in the same group, but everything else looks good
| <TargetFrameworks Condition="'$(BuildNet8)' == 'true'">net8.0</TargetFrameworks> | ||
| <TargetFrameworks Condition="'$(TargetFrameworks)' == ''">net481;net8.0</TargetFrameworks> | ||
| <OutputType>Library</OutputType> | ||
| <Nullable>enable</Nullable> |
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.
this breaks the CI build on .net framwork 4.8.1
| </PackageReference> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.13.0" /> | ||
| <PackageReference Include="xunit.v3" Version="3.2.0" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="3.1.5" /> |
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.
this might be a good idea if it works in the CI and all we needed is included in the new nuget👍
| @@ -0,0 +1,3 @@ | |||
| #!/bin/sh | |||
|
|
|||
| dotnet run --project EDSEditorGUI2 --framework net8.0 --property WarningLevel=0 "$@" | |||
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.
| dotnet run --project EDSEditorGUI2 --framework net8.0 --property WarningLevel=0 "$@" | |
| dotnet run --project EDSEditorGUI2 --property WarningLevel=0 "$@" |
I did not need --framework, its only needed if project uses multiple .net versions
| <PackageReference Include="xunit.v3" Version="3.2.0" /> | ||
| <PackageReference Include="xunit.runner.visualstudio" Version="3.1.5" /> | ||
| </ItemGroup> | ||
|
|
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.
| @@ -1,18 +1,17 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
|
|
|||
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.
| <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> | ||
| </PackageReference> | ||
| <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.13.0" /> | ||
| <PackageReference Include="xunit.v3" Version="3.2.0" /> |
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.
using xunit.v3 is a good idea, but we need to get it through the CI first to see if it works for all .net versions
| @@ -0,0 +1,3 @@ | |||
| #!/bin/sh | |||
|
|
|||
| dotnet run --project EDSEditorGUI2 --framework net8.0 --property WarningLevel=0 "$@" | |||
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.
| dotnet run --project EDSEditorGUI2 --framework net8.0 --property WarningLevel=0 "$@" | |
| dotnet run --project EDSEditorGUI2 --property WarningLevel=0 "$@" |
Changed the tests' csprojs (for it to run as written in the xunit documentation).
Made a few scripts to use the CLI, GUI and run the tests in a shell.
I made those changes to enable
myselfany posix user to run our projects.I executed all of the scripts and they worked (even with args for the CLI one) without printing any unnecessary information.
Resolve #190 (just partially, merging this PR shouldn't close the issue)
I have read the contributing guidelines, I agree to following them and I agree to the Developer's Certificate of Origin 1.1.