Remove Travis and use Github Actions #228
Conversation
|
My plan is to remove Travis. |
|
@neo-project/core Can we remove Travis now? |
|
We can remove travis, but we need the coverage report too, we can use coveralls.io, i think that is much better. |
|
Coveralls done. @shargon |
|
Not for this PR, but are we also going to look at using GitHub package registry instead of myget? |
.github/workflows/dotnetcore.yml
Outdated
| - name: Setup .NET Core | ||
| uses: actions/setup-dotnet@v1 | ||
| with: | ||
| dotnet-version: 3.0.100 |
There was a problem hiding this comment.
Instead of hard coding this in the yaml file where it only affects the build system, we should put this in a global.json file. I'm guessing there's a GH Action to pull the dotnet-version from global.json.
There was a problem hiding this comment.
I can't find a way right now. Maybe we improve it later.
There was a problem hiding this comment.
Strange. Azure Pipelines supports this today: https://docs.microsoft.com/en-us/azure/devops/pipelines/tasks/tool/dotnet-core-tool-installer?view=azure-devops
There was a problem hiding this comment.
The Github Actions doesn't support it. https://github.com/actions/setup-dotnet#usage
.github/workflows/dotnetcore.yml
Outdated
| - name: Setup NuGet.exe for use with actions | ||
| uses: NuGet/setup-nuget@v1 | ||
| - name: Pack with dotnet | ||
| run: dotnet pack -c Debug -o out --include-source --version-suffix CI$(date +%s) |
There was a problem hiding this comment.
Should you also --include-symbols?
There was a problem hiding this comment.
I found that if you add --include-source, there is no difference between adding or not adding --include-symbols.
.github/workflows/dotnetcore.yml
Outdated
| - name: Setup NuGet.exe for use with actions | ||
| uses: NuGet/setup-nuget@v1 | ||
| - name: Pack with dotnet | ||
| run: dotnet pack -c Debug -o out --include-source --version-suffix CI$(date +%s) |
There was a problem hiding this comment.
Instead of --include-source, consider using sourceLink
There was a problem hiding this comment.
I think with --include-source we have the same effect.
|
@neo-project/core @devhawk Ready to merge. |
|
|
||
| <PropertyGroup> | ||
| <Copyright>2016-2019 The Neo Project</Copyright> | ||
| <AssemblyTitle>Neo.VM</AssemblyTitle> | ||
| <Description>Neo.VM</Description> | ||
| <Version>3.0.0-preview1</Version> | ||
| <VersionPrefix>3.0.0</VersionPrefix> |
There was a problem hiding this comment.
@devhawk I agree with you that we could use Nerdbank, but I think this may be outside the scope of this PR.
| - name: Setup NuGet.exe for use with actions | ||
| uses: NuGet/setup-nuget@v1 | ||
| - name: Pack with dotnet | ||
| run: git rev-list --count HEAD |xargs printf "CI%05d" |xargs dotnet pack -c Debug -o out --include-source --version-suffix |
There was a problem hiding this comment.
using the git rev-list count is an improvement on using timestamp. However, it does mean that different branches can end up with the same version suffix. NerdBank gitversion solves this issue by including the commit hash in the version info for branches other than master + releases
There was a problem hiding this comment.
Hi @devhawk, maybe you could create an issue to discuss this tool? I saw your 'thumbs up' on Erik's comment and considered it your approval 😅 .
There was a problem hiding this comment.
Currently we only build and publish the master branch. So there is no reason to include the commit hash in the version info.
That seems like more work, vs. just setting NBGV up now and being done with it. But if that's the approach core dev wants to take, it's fine by me. |
|
I think the core dev approach is to make the best decision for the project. In fact, if 2 PRs will have the same height, we need to change this. |
|
Currently we only build and publish on the master branch. |
dotnet format.