-
Notifications
You must be signed in to change notification settings - Fork 95
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
build(csharp): script build, test, package and smoke test #1345
Conversation
…-adbc into dev/nuget-releases
…-adbc into dev/nuget-releases
|
csharp/scripts/Build-All.ps1
Outdated
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.
Where is this used?
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.
Also, other similar scripts have gone in ci/scripts (plus, there's already one there...)
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 not yet used in a pipeline. It is used locally right now.
@@ -10,6 +10,7 @@ ci/linux-packages/changelog | |||
ci/linux-packages/*.install | |||
csharp/*.sln | |||
csharp/*.csproj | |||
csharp/*.props |
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.
.props files are included in .csproj files and cannot have comments at the top
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.
The existing bash scripts separate build/pack and do not have test. Can we be consistent with the powershell and bash setup?
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 moved some around to align with the bash scripts but the csharp_smoketest script still has a different behavior than what any other script does.
is there anything else needed to merge this @lidavidm or @CurtHagenlocher? |
The C# changes look good to me. I feel less qualified to comment on the build changes. |
I'm fairly backlogged - I'll try to review in earnest this week but it may be a while |
I'm going to check in the purely C# changes and the build system changes can be submitted as a smaller PR. |
Adding the build scripts that were split out from #1345 --------- Co-authored-by: David Coe <[email protected]>
Down payment toward #1121: