Skip to content

Update to NET 8 SDK#422

Merged
lahma merged 1 commit intosebastienros:mainfrom
lahma:net8
Nov 22, 2023
Merged

Update to NET 8 SDK#422
lahma merged 1 commit intosebastienros:mainfrom
lahma:net8

Conversation

@lahma
Copy link
Collaborator

@lahma lahma commented Nov 22, 2023

Ensures consistent test path resolution when UseArtifactsOutput is in use.

@lahma lahma merged commit 9b2487b into sebastienros:main Nov 22, 2023
@lahma lahma deleted the net8 branch November 22, 2023 06:51
@@ -0,0 +1,6 @@
{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You agree this file is not required, or am I missing something?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even more than when the next OS image is updated and doesn't have this one it would break.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well you are missing the fact that the solution does not require NET 8 SDK because nothing targets net8.0 yet. This will lead to the fact that depending the SDK you have (say 6 or 8) you will get different output folders (UseArtifactsOutput) and the tests will break.

So when the targets will be updated to net8.0 it will implicitly require the SDK and this file can be removed. But I discovered that there's regex changes that break the tests when run under net8.0 so requires some work.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is to enfore that 8.0 sdk is used because of UseArtifactsOutput, ok.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for the time being. A useful features but can break things if solution is used with older SDK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants