-
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
new build scripts for release pipeline #1442
Conversation
} | ||
|
||
Pop-Location | ||
} | ||
} | ||
|
||
function New-Catalog | ||
{ | ||
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseCompatibleCommands', '')] |
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 there a specific diagnostic here that was appearing?
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.
One of the stages in the sign build was to run all the tests, and one of the tests found new-filecatalog not being present in all versions, but I hadn't picked up the latest master. Also, I removed the test phase from the build since it happens in CI. I can pull this line.
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.
nope - I need to put it back - now it's failing in the CI 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 worries -- perhaps we can open an issue to track removing this?
build.psm1
Outdated
$framework, | ||
"--configuration", | ||
"$buildConfiguration" | ||
if ( $env:InVSTS ) { |
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 TF_BUILD
environment variable is defined in Azure DevOps and could be used for that instead of defining a custom variable on the release build. Or do you need to specifically know if you are running in the release build? If so, why do we need to build differently with a different output directory when we copy it to the module directory anyway at the end?
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'll try that - thanks!
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 more important question to understand for me is why we cannot keep the existing directory structure?
@@ -163,27 +165,36 @@ function Start-ScriptAnalyzerBuild | |||
$foundVersion = Get-InstalledCLIVersion | |||
Write-Warning "No suitable dotnet CLI found, requires version '$requiredVersion' found only '$foundVersion'" | |||
} | |||
$verboseWanted = $false |
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 is this needed? PowerShell has the $VerbosePreference variable for that and should automatically propagate the usage of -Verbose
to down to the other called cmdlets (splatting is the only exception where this does not work as far as I am aware)
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.
because I want finer grained control, I don't want verbose everywhere. I tried it without and the logs are super large and I would rather be more targeted (no reason to be verbose on import-module for example).
incorporate catalog building into module
…filecatalog exists rather than IsWindows variable which doesn't exist on PS5
Remove comment code change logic to use existing environment variable when building in VSTS Remove extraneous suppression of New-FileCatalog
} | ||
|
||
Pop-Location | ||
} | ||
} | ||
|
||
function New-Catalog | ||
{ | ||
[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseCompatibleCommands', '')] |
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 worries -- perhaps we can open an issue to track removing this?
PR Summary
The previous build scripts for release used Docker which is no longer available for MS build/signing pipelines. These needed to be rewritten them to not use docker and I created some additional configuration scripts for signing. There shouldn't be any affect on the product. I made a slight improvement to one of the tests so when it fails we get better diagnosability.
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.