Update SDKs and use C# 8.0 #872
Conversation
|
Waiting for the VS2019 images to land on CI. |
src/ImageSharp/ImageSharp.csproj
Outdated
| <DebugSymbols>True</DebugSymbols> | ||
| <Features>IOperation</Features> | ||
| <LangVersion>7.3</LangVersion> | ||
| <LangVersion>preview</LangVersion> |
There was a problem hiding this comment.
This could be an issue.
There was a problem hiding this comment.
This works with the v2.2.202 of the dotnet SDK that just got published. And will also work when the Visual Studio 2019 images go live.
There was a problem hiding this comment.
@dlemstra noticed an issue with the "wildcard" settings. Can't remember what it was, but we decided to go explicit. Even without understanding it, it seems safer to me if we want interop across different IDE-s / SDK-s.
|
I pressed "ready for review" accidentally. |
appveyor.yml
Outdated
| @@ -1,5 +1,5 @@ | |||
| version: 1.0.0.{build} | |||
| image: Visual Studio 2017 | |||
| image: Visual Studio 2019 Preview | |||
There was a problem hiding this comment.
This is just an enabler for C# 8.0, leaving target_framework compatibility untouched. Is my understanding correct?
There was a problem hiding this comment.
Yep. We don't need to touch the target frameworks. I have things compiling locally -- just waiting on AppVeyor to update now.
There was a problem hiding this comment.
Things look good on Travis.
iamcarbon
changed the title
| // The compression value in OS/2 bitmap has a different meaning than in windows bitmaps. | ||
| // Map the OS/2 value to the windows values. | ||
| switch (compression) | ||
| infoHeader.Compression = compression switch |
|
And we've landed! |
Codecov Report
@@ Coverage Diff @@
## master #872 +/- ##
==========================================
- Coverage 88.97% 88.94% -0.03%
==========================================
Files 1018 1018
Lines 44579 44543 -36
Branches 3234 3245 +11
==========================================
- Hits 39663 39620 -43
+ Misses 4195 4187 -8
- Partials 721 736 +15
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #872 +/- ##
==========================================
- Coverage 89.02% 88.98% -0.04%
==========================================
Files 1024 1024
Lines 44786 44792 +6
Branches 3230 3255 +25
==========================================
- Hits 39872 39860 -12
+ Misses 4202 4195 -7
- Partials 712 737 +25
Continue to review full report at Codecov.
|
|
And now travis is broke. Hahah. EDIT: Seems to be a known bug. Going to try a different Linux version. |
|
Success! Ready for review. CC @JimBobSquarePants @antonfirsov This enables use to use static local functions, switch expressions, nullable reference types, and disposable ref structs. Some additional features are enabled using the preview lang-version but I think these are still being worked on and will be enabled in an upcoming SDK. |
iamcarbon
changed the title
There was a problem hiding this comment.
I think we need to get the VS2019 part merged ASAP, but I have a simple concern regarding C# 8.0:
what if other contributors (with open PR-s) haven't switched yet?
Let's think this over, maybe it would be wiser to wait a few more weeks with the language version change!
@dlemstra @tocsoft @brianpopow @vpenades @jongleur1983 @Sergio0694 please share your thoughts!
|
@antonfirsov I'm still using 2017. At home I'll probably switch to 2019 next week. At work it'll take a couple months. But it's fine since I'm only doing things with the source code of imagesharp at home. I know some people that is waiting to switch to 2019 when NetCore 3 is released. This makes some sense since 2019 is required for NetCore 3, and it's what people actually want to target. Another matter is if you're exposing C# 8 characteristics in the public API. In my experience, I've been using C# 7.3 "in" and "unmanaged" keywords. Nuget packages that target C# 7.3 can be consumed by c# 7.0 projects as long as you don't call any method using higher level features, in that case, it will throw an exception. |
|
I already have VS2019 + .NET Core 3.0 preview on my PC, so this wouldn't be a breaking change for me personally (as far as working on the repo itself goes). But:
Just thinking out loud here 😄 |
|
I am one of the people that will switch to 2019 after NetCore 3 has been released. But if there are more people who will want to switch already then feel free to do so. If the switch is made earlier I can always spin up a VM in Azure and install VS2019 on there and work from there. |
|
I'm reading a little confusion here... .NET Core 3.0 and C#8 should be considered independantly. VS2019 does not mean we require .NET Core 3.0 nor should it be a consideration when choosing to move from 2017. Bug fixes, faster compilation, better refactorings etc should be your consideration here. We also should not be concerned by certain C#8 features not being availabe on .NET Framework. We specifically target the framework so if something doesn't work the build will simply fail. That said, I agree with @antonfirsov here. I think we should split this PR into two.
@tannergooding You're our man on the inside. Does this make sense to you? |
JimBobSquarePants
left a comment
JimBobSquarePants
left a comment
There was a problem hiding this comment.
See below for my thought. Thanks!
|
@JimBobSquarePants, that makes sense to me. VS2019 is released and supported, you should feel free to move to it as you would any new version. Community edition can also be used for contributing to open source projects, so I don't believe moving to it will block anyone. C# 8 and .NET Core 3 are both still in preview and being worked on. If you want to support these, there are ways you can configure the infrastructure to require a minimum SDK version already be on box or for your |
|
I still don't use ImageSharp in any projects, just contributing for fun. I'm on VS2019 at home and at work already, but did not yet switch to C#8 anywhere (but I'm fine with doing so). |
|
@JimBobSquarePants In case that "I see some confusion here" also referred to my answer, just to clarify, what I meant is that while I'm aware the VS2019 is completely independent from .NET Core 3.0 and C# 8.0, from what I saw lots of users are just waiting for .NET Core 3.0 to be publicly released before bothering to upgrade from VS2017. Then of course, you could still go ahead with the C# 8.0 integration and eventually just force those users to upgrade to VS2019 now, I'm not saying that'd be a bad thing per se 😄 As far as I'm concerned, my only fear regarding C# 8.0 is how it'll play out with the .NET Native compiler on UWP, which as you well know, already just fails to compile ImageSharp. |
|
I am using VS2019 at home and at work, so i am fine with that. I also have no objection in switching to C# 8.0 |
|
@JimBobSquarePants wow, actually I wanted to accept the changes based on the positive responses from the community, but let's do the split then! 😄 I will open a separate PR copying the SDK changes from this one very soon. Update: Done in #891 |
|
Let's revisit this again soon. |
|
Thanks all for your contribution to the discussion. It's really great to read your input! @iamcarbon I'm very keen on moving to C# 8. There's some stuff in there that I really like the look of that could be very useful to us. I just want to be cautious and work with a proper plan. ImageSharp is the end of a dependency chain and I think we should begin any migration efforts at Core so we can dogfood our changes as they propegate through that chain. |
9 participants
Prerequisites
Description