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

Modernise for newer target frameworks #44

Merged
merged 7 commits into from
Dec 13, 2020
Merged

Modernise for newer target frameworks #44

merged 7 commits into from
Dec 13, 2020

Conversation

jetersen
Copy link
Contributor

@jetersen jetersen commented Nov 18, 2020

  • Target newer frameworks including net5 and netstandard2.1 for greater compatibility
  • Include symbols package
  • Use License Expression
  • Add Package References for reference assemblies for NET Framework
  • Add SourceLink
  • Add GitVersion Properties
  • Use GitHub Actions
  • Remove legacy global.json
  • Remove .travis.yml
  • Remove Dockerfile
  • Remove Makefile as it was only used for docker tests

Hi @omar
Would you prefer to use license file over license expression?

I created GitHub action that will push to nuget.org when you publish a GitHub Release, as long as you add an NuGet API key to the GitHub repo secrets with the NUGET_API_KEY name.
The action uses GitVersion to retrieve version based on git tags, so no need to make changes to csproj file to deploy a new version, simply create a GitHub Release and the rest will be taken care of.

I have used the same GitHub action for https://github.com/nmklotas/GitLabApiClient and https://github.com/Specshell/specshell.software.ndde

Reason to target net standard 2.0 and 2.1 can be found here: https://docs.microsoft.com/en-us/dotnet/standard/frameworks#latest-versions

- Target newer frameworks including net5 and netstandard2.1 for greater compatibility
- Include symbols package
- Use License Expression
- Add Package References for reference assemblies for NET Framework
- Add SourceLink
- Add GitVersion Properties
- Use GitHub Actions
- Remove legacy global.json
- Remove .travis.yml
- Remove Dockerfile
- Remove Makefile as it was only used for docker tests
noted that test failed on my machine using danish culture info
@jetersen
Copy link
Contributor Author

Seems the project does not allow GitHub Actions to run.
You can enable them here

I have the build running here: https://github.com/jetersen/ByteSize/runs/1420624006

@omar
Copy link
Owner

omar commented Dec 13, 2020

@jetersen thank you for putting this PR together. This all looks great. One question I had was about the supported framework. As I understand it, targeting netstandard1.0 would give us the widest support of customers. If we move things to a minimum of netstandard2.0, we would lose support for previous frameworks. That's something I'd like to avoid given there's no immediate need to require netstandard 2.0

@jetersen
Copy link
Contributor Author

@omar sounds reasonable 👍

Comment on lines +30 to +38
- name: Setup dotnet
uses: actions/setup-dotnet@v1
with:
dotnet-version: '3.1'

- name: Setup dotnet
uses: actions/setup-dotnet@v1
with:
dotnet-version: '5.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI these lines can be removed once this lands: actions/runner-images#1891

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@omar omar merged commit bf739ee into omar:master Dec 13, 2020
@jetersen jetersen deleted the net5 branch December 13, 2020 17:35
@jetersen
Copy link
Contributor Author

@omar you'd want to enable the GitHub actions here: https://github.com/omar/ByteSize/settings/actions

@omar
Copy link
Owner

omar commented Dec 14, 2020

I have that enabled. Still not seeing anything running.
image

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