-
Notifications
You must be signed in to change notification settings - Fork 44
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
Build with .NET 8 #580
Build with .NET 8 #580
Conversation
# Conflicts: # source/Halibut/Transport/SecureListener.cs
@@ -108,7 +108,7 @@ ITargetDefinition CompileDefinition(ITargetDefinition targetDefinition, [CanBeNu | |||
Target TestWindowsNet48 => _ => TestDefinition(_, CompileNet48, "net48", runDotMemoryTests: true); | |||
|
|||
[PublicAPI] | |||
Target TestWindowsNet60 => _ => TestDefinition(_, CompileNet60, "net6.0", runDotMemoryTests: true); | |||
Target TestWindowsNet80 => _ => TestDefinition(_, CompileNet80, "net8.0", runDotMemoryTests: true); |
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 think we would want to keep build net6.0 and test net6.0 and then add the net8.0 versions.
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, I'd been meaning to run this PR through with someone from SaST to talk about things like this!
@@ -51,10 +51,11 @@ | |||
<PackageReference Include="Serilog.Sinks.ColoredConsole" Version="3.0.1" /> | |||
</ItemGroup> | |||
|
|||
<PropertyGroup Condition=" '$(TargetFramework)' == 'net48' "> | |||
<PropertyGroup Condition=" '$(TargetFramework)' == 'net4.8' "> |
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 correct?
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.
It's wrong, it should be "net48". Perils of putting down a PR for a week or two then picking it back up again, sorry
<DefineConstants>$(DefineConstants);SUPPORTS_WEB_SOCKET_CLIENT;DOES_NOT_SUPPORT_CANCELLATION_ON_SOCKETS</DefineConstants> | ||
</PropertyGroup> | ||
|
||
<PropertyGroup Condition=" '$(TargetFramework)' == 'net6.0' Or '$(TargetFramework)' == 'net8.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.
Double up?
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 general plan we've been following is:
- Switch the compilers to use .NET 8
- Build the library only targeting .NET 6 unless there's some special reason not to
- Have the tests run on both .NET6 and .NET8
But Halibut is complicated and messy, let's have a chat about the strategy for it
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.
@@ -8,10 +8,11 @@ | |||
</PropertyGroup> | |||
|
|||
<PropertyGroup Condition="!$([MSBuild]::IsOSUnixLike())"> | |||
<TargetFrameworks>net48;net6.0;net8.0</TargetFrameworks> |
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.
Double up
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.
net6.0 and net8.0 aren't double-ups?
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.
Are you a customer of Octopus Deploy? Please contact our support team so we can triage your PR, so that we can make sure it's handled appropriately.
Background
Results
Fixes https://github.com/OctopusDeploy/Issues/issues/... (optional public issue)
Fixes https://github.com/OctopusDeploy/ResearchAndDevelopment/issues/... (optional private issue)
See How we use GitHub Issues (including this flowchart
Before
After
How to review this PR
Quality ✔️
Pre-requisites