-
Notifications
You must be signed in to change notification settings - Fork 382
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
WIP: get Linux build to upload build artifacts #1293
base: main
Are you sure you want to change the base?
Conversation
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.
Looks good to me, just some minor changes need to be done before the merge. Thanks for investigating and fixing it
else { | ||
$dotnet = "dotnet" | ||
} | ||
& $dotnet publish -f $Framework -c $Configuration |
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.
Was this the fix for the premature build abortion
#if ($IsLinux) | ||
#{ | ||
# $expectedNumRules = 4 | ||
#} |
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 should just delete those commented out code lines now that it works without a special case (not sure why, I guess some upstream dependency received a fix)
# PowerShellEdition: WindowsPowerShell | ||
# - APPVEYOR_BUILD_WORKER_IMAGE: WMF 4 | ||
# PowerShellEdition: WindowsPowerShell | ||
# PSVersion: 4 |
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.
Please uncomment again before the merge
if ( $LASTEXITCODE -ne 0 ) { | ||
$buildOutput | Write-Verbose -Verbose | ||
throw "$buildOutput" | ||
} |
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.
Indentation of closing brace is 1 level too much
Write-Verbose -Verbose "bootstrap complete: $(get-date)" | ||
} | ||
catch { | ||
Write-Warning "error in invocation of build.ps1 from Invoke-AppVeyorInstall" |
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 catch the error and not let it error out?
Close and re-open to retrigger CI to check this still works |
@JamesWTruher Shall we close this one as #1320 has the essential bits of this PR (fixing the build and uploading the tests work now again), the only thing that this PR would add are the additional verbose logs |
Although the recently merged PR fixed the Ubuntu build it seems that it has lasted only a few days as AppVeyor updated their image with breaking changes again, will close and re-open to see if this branch is any different |
PR Summary
get Linux build to upload build artifacts
Add additional output to assist with debugging if there is a problem
PR Checklist
.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
to the beginning of the title and remove the prefix when the PR is ready.