-
Notifications
You must be signed in to change notification settings - Fork 2
Adapt build.cake for teamcity. Make project buildable, add 'Fortis.' … #2
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
base: develop
Are you sure you want to change the base?
Conversation
…suffix to nugets. Adapt tests for Teamcity.
| <ItemGroup> | ||
| <PackageReference Include="NUnit3TestAdapter" Version="3.13.0" /> | ||
| <PackageReference Include="System.Data.SQLite" Version="1.0.109.1" /> | ||
| <PackageReference Include="System.Data.SQLite" Version="1.0.112.2" /> |
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.
Why?
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.
I don't think that we need to update versions without necessary
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.
Could not build with this version and also could not find it at nuget.org.
Now it works. Somehow related to our local nuget feed temporarty issues
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 is unlisted package, but it exists on nuget.org. Upstream appveyor biuld it (me too). So I disagree with version update.
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.
I see it now. Ok
| { | ||
| ArgumentCustomization = aggs => aggs.Append(GetDotNetCoreArgsVersions()) | ||
| }); | ||
| DotNetCoreRestore( |
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.
Could you revert non significant changes: we will get troubles with upstream to merge this file back if we don't push these changes to upstream.
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.
Here (and in other files)
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.
I don't think we are gonna merge this back. We have got completely different CI.
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.
If so - create different file.
Please note, that this file can be used for local build.
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.
Regardless of merge and merge back: please don't change nonsignificant whitespace in your PRs - it is non significant change and these type of change made PR bit more non readable.
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.
Sorry for that. Just tried to make it more readble. I also replaced all tabs to whitespaces.
| <DebugType>portable</DebugType> | ||
| <SymbolPackageFormat>snupkg</SymbolPackageFormat> | ||
| <AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb</AllowedOutputExtensionsInPackageBuildOutputFolder> | ||
| <PackageID>FortisOnline.$(MSBuildProjectName)</PackageID> |
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.
Is it possible to get prefix somewhere outside?
My idea is push these changes to upstream to allow other contributors to create own teamcity build.
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.
No. It's not. Only in csproj files or here
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.
Or maybe, replace this property in cake, at build time...
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.
I think you should have discussed your ideas beforehand.
May be it worth creating separate cake file for TeamCity then
| }); | ||
|
|
||
| // ===================================================================================================== | ||
| Task("ValidateSourceLink") |
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.
Can we enhance existing cacke file without removing a lot of existing code?
Is this file still work in appVeyor?
Is this file usefull for local builds?
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.
I removed all AppVeyor stuff in purpouse - to keep cake-file clean. We dont't have AppVeyor
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.
We don't have AppVeyor, but others and upstream has. We will get merge conflicts when upstream updates this file.
So I suggest:
- to create different file for our local teamcity (and reuse all possible in both files)
- or insert code necessary to teamcity into existing pipeline
…suffix to nugets. Adapt tests for Teamcity.