Skip to content
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

Enable .NET 6 Preview Builds #1745

Merged
merged 21 commits into from
Aug 22, 2021
Merged

Enable .NET 6 Preview Builds #1745

merged 21 commits into from
Aug 22, 2021

Conversation

JimBobSquarePants
Copy link
Member

@JimBobSquarePants JimBobSquarePants commented Aug 19, 2021

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Just use the environment compiler to build things. When consumers build their applications they're already consuming a prebuilt binary. We only need to test with individual frameworks.

Added .NET 6 Preview builds to the matrix and fixed up the solution to handle that scenario. Also fixed the langversion to C#9

This allows us to use features like nint and also build/test against .NET 6 previews which we sorely need to do.

ci-build.ps1 Outdated
dotnet clean -c Release

$repositoryUrl = "https://github.com/$env:GITHUB_REPOSITORY"

# Building for a specific framework.
dotnet build -c Release -f $targetFramework /p:RepositoryUrl=$repositoryUrl
Copy link
Member

Choose a reason for hiding this comment

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

This will build every target making the builds longer, won't this?

Does the target framework impact the compiler version, isn't it purely an SDK thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep so I had this badly wrong. My understanding was that specifying the target framework moniker causes the underlying MsBuild call to look for and use that specific SDK to compile against. That's obviously not true.

I've fixed up the project now so we can build against the preview in CI and everything else is unaffected. I got the langversion working properly also so we can use the C#9 features.

@codecov
Copy link

codecov bot commented Aug 19, 2021

Codecov Report

Merging #1745 (4ba43aa) into master (9b09775) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1745   +/-   ##
=======================================
  Coverage   84.46%   84.46%           
=======================================
  Files         836      836           
  Lines       36737    36737           
  Branches     4306     4306           
=======================================
  Hits        31029    31029           
  Misses       4876     4876           
  Partials      832      832           
Flag Coverage Δ
unittests 84.46% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ogramEqualizationSlidingWindowProcessor{TPixel}.cs 98.91% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b09775...4ba43aa. Read the comment docs.

@JimBobSquarePants JimBobSquarePants changed the title Use environment compiler version to build Enable .NET 6 Preview Builds Aug 20, 2021
@JimBobSquarePants
Copy link
Member Author

Think I might just flatten this one when I merge it 😝

@JimBobSquarePants JimBobSquarePants added this to the 2.0.0 milestone Aug 20, 2021
@iamcarbon
Copy link
Contributor

We should consider dropping netcoreapp2.1 for the 2.0 release. It's going out of support this month.

@JimBobSquarePants
Copy link
Member Author

@iamcarbon It’s only V2.0 because we have to introduce the breaking changes in #1730

My plan following this release is to immediately start work on V3.0 which will reduce the target frameworks to a minimum of .NET Core 3.1

As such this will act as the last stable release for legacy platforms.

@JimBobSquarePants JimBobSquarePants requested a review from a team August 22, 2021 09:17
@@ -257,7 +257,7 @@ protected override void OnFrameApply(ImageFrame<TPixel> source)
[MethodImpl(InliningOptions.ShortMethod)]
private void AddPixelsToHistogram(ref Vector4 greyValuesBase, ref int histogramBase, int luminanceLevels, int length)
{
for (int idx = 0; idx < length; idx++)
for (nint idx = 0; idx < length; idx++)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@JimBobSquarePants could you explain a bit what the advantages of using nint is? This is kind of new to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically better codegen.

#1734 (comment)

Copy link
Collaborator

@brianpopow brianpopow left a comment

Choose a reason for hiding this comment

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

looks good

@JimBobSquarePants JimBobSquarePants merged commit 36f4ad4 into master Aug 22, 2021
@JimBobSquarePants JimBobSquarePants deleted the js/default-build branch August 22, 2021 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants