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

Update to latest .Net Core runtime #33

Closed
wants to merge 1 commit into from
Closed

Conversation

secana
Copy link

@secana secana commented Jun 10, 2018

Description

Updated the .Net Core template to the latest runtime and sdk version.

Motivation and Context

All new .Net Core project created are automatically created with the new version 2.1 meaning that no new .Net Core project runs in the current template.

Furthermore the new version brings significant performance improvements at build and runtime.

  • I have raised an issue to propose this change (required)

How Has This Been Tested?

Build the new image to check if everything works as expected.
Ran the image and check that the needed files are there.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [ x] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@derek
Copy link

derek bot commented Jun 10, 2018

Thank you for your contribution. I've just checked and your commit doesn't appear to be signed-off.
That's something we need before your Pull Request can be merged. Please see our contributing guide.

@derek derek bot added the no-dco label Jun 10, 2018
@alexellis
Copy link
Member

Hi @secana that sounds like a good move. Thanks for proposing the change.

Let me ping one or two people about this to make sure we are not introducing any breaking changes.

We can make this change in the project, or use your pull request, but you will need to sign off the commit. Signing-off is described in the contribution guide linked to by Derek.

Let me know if you'd like to join the Slack community too.

Alex

@burtonr @rorpage

@alexellis
Copy link
Member

@secana I also want to connect you with @burtonr who has been working on a Kestrel template for a much higher throughput..

https://github.com/BurtonR/csharp-kestrel-template

The other .NET expert is @fpommerening 😄

@burtonr
Copy link
Contributor

burtonr commented Jun 10, 2018

There were no breaking changes in the .NET Core 2.1 release, so this should be a safe upgrade.
They did add some performance improvements which are always a good thing!
Here is the announcement post from Microsoft

@secana once you --amend your commit with a sign-off I think we're good to upgrade!
Verified this with my C# functions locally. 👍

Copy link

@rorpage rorpage left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@alexellis
Copy link
Member

@rorpage did you test this change?

@alexellis
Copy link
Member

@secana can you please see the comments? We are thankful for your change but need you to follow the process for contributions. If we don’t hear by the end of the week we’ll close this PR and re-do the work, so it would be great if you could follow-up.

Alex

@alexellis
Copy link
Member

@secana it's been around 6 weeks since this PR was raised without completing a "DCO" sign-off. I think we can assume someone in the community needs to take over the work now so I'll close the PR and raise an issue for it.

If you decide to revisit the contribution guide and follow the above please just re-open the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants