-
Notifications
You must be signed in to change notification settings - Fork 35
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
.NET 5.0 with C# 9 #123
.NET 5.0 with C# 9 #123
Conversation
1c1b030
to
8169c00
Compare
<Project> | ||
<PropertyGroup> | ||
<!-- Compile Include="x.cs" doesn't use LangVersion defined in the PropertyGroup --> | ||
<LangVersion>9.0</LangVersion> |
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'd prefer to keep the setting either in csproj or in \Build\Props\CodeJam.Default.props
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.
The problem is that it doesn't help.
Files such as <Compile Include="JetBrains.Annotations.cs"/>
don't receive LangVersion 9.0.
I submitted a bug.
It it possible to include a file from Build\Props... , but then there is no difference between setting it here or including.
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 simply didn't find other working solution.
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.
Aha! Let's remove <LangVersion>
from csproj files then
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.
Ok.
Btw, you can vote for the issue: https://developercommunity2.visualstudio.com/t/itemgroup-compile-include-doesnt-use-the-same-lang/1255582?from=email&preview=true
Build/Props/CodeJam.Targeting.props
Outdated
@@ -11,8 +11,8 @@ | |||
<PropertyGroup> | |||
<CopyMeMinimalTargetFrameworks>net461;netstandard2.0</CopyMeMinimalTargetFrameworks> | |||
<CopyMeMinimalTestTargetFrameworks>net461;netcoreapp2.0</CopyMeMinimalTestTargetFrameworks> | |||
<CopyMeTargetFrameworks>net472;net461;net45;net40;net35;net20;netstandard2.1;netstandard2.0;netstandard1.5;netstandard1.3;netcoreapp3.0;netcoreapp2.0;netcoreapp1.0</CopyMeTargetFrameworks> | |||
<CopyMeTestTargetFrameworks>net48;net472;net471;net47;net461;net45;net40;net35;net20;netcoreapp3.0;netcoreapp2.1;netcoreapp1.1</CopyMeTestTargetFrameworks> | |||
<CopyMeTargetFrameworks>net50;net472;net461;net45;net40;net35;net20;netstandard2.1;netstandard2.0;netstandard1.5;netstandard1.3;netcoreapp3.0;netcoreapp2.0;netcoreapp1.0</CopyMeTargetFrameworks> |
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.
We need to regenerate FW monikers in CodeJam.TargetingTests.EmitTargetingConstants
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.
Sure, will add there.
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.
Actually didn't know it is autogenerated :)
Why not T4 as done in other places ?
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.
Let's fix it after this PR.
@NN--- Thanks for the great work!) |
As suggestion: #pragma warning disable SYSLIB0012 // 'Assembly.CodeBase' is obsolete: 'Assembly.CodeBase and Assembly.EscapedCodeBase are only included for .NET Framework compatibility. Use Assembly.Location instead.
var uri = new Uri(assembly.CodeBase);
#pragma warning restore SYSLIB0012 I think we should follow the recommendation and use CodeBase only when targeting to the net FW |
I'm not sure but this looks kinda strange for me. |
As for me, everything else is ok. One more time, a great thanks you! :) |
@ig-sinicyn Next part is to
.NET 5.0 added ObsoleteAttribute to this :) |
Wow, nice catch! |
@ig-sinicyn AppVeyor still don't have VS 16.8 , hence the preview version. |
Fixed comments. Good enough to commit ?:) |
Yep, LGTM 👍 |
Thanks. |
Do you know why the build is so slow ? |
Have no idea |
Why the build failed ? |
2163b00
to
c99164d
Compare
nunit3-console doesn't support net50 yet. |
@ig-sinicyn Thanks for approval. |
Here is what fails. |
Found the relevant documentation: https://docs.microsoft.com/en-us/dotnet/standard/base-types/string-comparison-net-5-plus |
@ig-sinicyn any idea how to test invariant culture ? |
Feel free to disable tests (with some comment, of course). UP. #125 |
I made test which compares ss with ß , validating invariant comparison is used. |
Just remove |
ea9570f
to
d81ee50
Compare
Found the problem. AppVeyor has .NET 5.0 preview which defined NETCOREAPP3_1 for .NET 5.0 :( |
Merge after #124