Skip to content

Updated TFM of Notifications package from netcoreapp3.0 to netcoreapp3.1. #3383

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

Merged
merged 3 commits into from
Jul 22, 2020

Conversation

azchohfi
Copy link
Contributor

@azchohfi azchohfi commented Jul 8, 2020

Fixes #3382

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Build or CI related changes

What is the current behavior?

Microsoft.Toolkit.Uwp.Notifications targets an end of life SDK (netcoreapp3.0). We should upgrade it to netcoreapp3.1.

What is the new behavior?

Microsoft.Toolkit.Uwp.Notifications targets netcoreapp3.1.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

I've also fixed a small race condition on the tests, since they run on parallel, per class, and two classes/tests were writing to the same file, which would eventually fail.

@ghost
Copy link

ghost commented Jul 8, 2020

Thanks azchohfi for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost assigned Kyaa-dost Jul 8, 2020
@azchohfi azchohfi changed the title Updated TFM of Notifications package from netcoreapp3.0 to netcoreapp… Updated TFM of Notifications package from netcoreapp3.0 to netcoreapp3.1. Jul 8, 2020
@azchohfi
Copy link
Contributor Author

azchohfi commented Jul 8, 2020

Interesting that even removing the installation of that sdk, it is still warning about it:

##[warning].NET Core SDK/runtime 2.2 and 3.0 are now End of Line(EOL) and have been removed from all agents. If you're using these SDK/runtimes, kindly upgrade to newer versions which are not EOL, or else use UseDotNet task to install the requied version.

Its still building, and I think this is just a warning to make sure people see it, since once they remove it it might break builds.
As you can see here:
https://github.com/actions/virtual-environments/blob/master/images/win/Windows2019-Readme.md#net-core-sdk
The SDK we were manually installing was already installed (on a system level, on the build machines), so it shouldn't make any difference. At least, with this PR, we moved away from the unsupported SDK and we are speeding up our builds by ~45seconds, so its a win anyway.

Copy link
Member

@Sergio0694 Sergio0694 left a comment

Choose a reason for hiding this comment

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

Not familiar with the WPF-specific stuff, but the .NET Core and NuGet updates look good! 👍

@@ -19,6 +19,11 @@
<DefineConstants Condition="'$(DisableImplicitFrameworkDefines)' != 'true'">$(DefineConstants);WINRT</DefineConstants>
</PropertyGroup>

<PropertyGroup Condition="'$(TargetFramework)' == 'net462'">
Copy link
Member

Choose a reason for hiding this comment

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

Woah, hadn't noticed the toolkit was still supporting .NET Framework 4.6.2 too! 😄

@azchohfi
Copy link
Contributor Author

azchohfi commented Jul 8, 2020

I did those updates since, for some bizarre reason, the Pages' *.g.cs were not being added properly when I did this change (no idea why). This small change is now using the UseWpf property(which makes them build fine), and I'm turning the automatic inclusion of pages off, since the Directory.Build.props is already adding them:
https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/5d03cac977f8e75a91b277066d406458c69f223f/Directory.Build.props#L81-L82
I've also removed the implicit inclusion of a few binaries since the UseWpf will now include them automatically.

@azchohfi
Copy link
Contributor Author

@azchohfi azchohfi merged commit f732fc1 into master Jul 22, 2020
@delete-merged-branch delete-merged-branch bot deleted the nonEOLNetCore branch July 22, 2020 02:19
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.

Switch to non-EOL .NET Core SDKs in the CI
4 participants