-
Notifications
You must be signed in to change notification settings - Fork 1
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
Publish as .NET Core Global Tool #1
Conversation
Unit test projects have to stay on net461 for now due to some of the libraries they consume.
This uses a version of LibGit2Sharp.NativeBinaries that works on the latest official microsoft/dotnet docker images.
The folder is no longer used and caused a warning during the build.
@@ -140,12 +140,12 @@ Task("Run-Tests") | |||
.IsDependentOn("DogfoodBuild") | |||
.Does(() => | |||
{ | |||
var settings = new DotNetCoreTestSettings | |||
{ | |||
var settings = new DotNetCoreTestSettings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of whitespace changes here makes this diff pretty noisy!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I revert the whitespaces I fixed, or keep the changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's up to you, it's easier to review when any "formatting" changes or "beautification" is kept entirely seperate imho, but it's not a blocker from my personal perspective. (Mainly because I am also often guilty of this :-))
@arturcic - for the new global tool, in addition to the cake build script (which builds and uploads a bunch of assets to appveyor) there is also a cake deploy script which basically downloads those assets from appveyor, and then publishes them to nuget.org and the like - you will want to amend that one too to include publishin of this new tool :-) |
I will do that as well, thanks |
I have added the publish task, but not able to test it, as it uses the Github Release Artifcats, so I commented the call to the task for now |
0d4adac
to
58ab04f
Compare
I'm sorry I totally missed this somehow. I rebased my branch today to get the latest from master as well and force pushed that thinking nobody was using my branch but myself. :) |
That's fine, I'll rebase my changes so that we can have the tool included into the netstandard2 PR |
Oh great, thanks for merging |
I tried to fix it on my end... but admittedly I'm not sure what I'm doing. :) I see your commits on my PR 1422 now but I also see two copies of my commits. (shrug) I'll see if I can clean it up some more. |
Looks good so far, the builds have passed |
I have merged the master branch into your branch so that we have those changes as well and I have also implemented the GitTools#1464.
Do you think we should include the GitVersion global tool into the nestandard2 PR or have a separate PR?