-
Notifications
You must be signed in to change notification settings - Fork 318
Revert "Narrowly fix Test-Packages.ps1 and Analyze-Code.ps1" #3112
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
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.
Pull Request Overview
This PR reverts a previous change that added error handling with Write-Error calls to PowerShell scripts. The revert removes the explicit error checking and Write-Error statements that were intended to make the scripts exit immediately on failure.
Key changes:
- Removes manual error checking after each
Invoke-LoggedCommandcall - Eliminates
Write-Errorstatements and intermediate$commandvariables - Returns to direct
Invoke-LoggedCommandcalls without explicit error handling
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| eng/scripts/Test-Packages.ps1 | Reverted cargo build and test commands to use direct Invoke-LoggedCommand calls without manual error checking |
| eng/scripts/Analyze-Code.ps1 | Reverted all cargo commands and dependency verification to use direct Invoke-LoggedCommand calls without manual error checking |
|
@heaths, @hallipr -- This PR reverts the narrow fix from last week after I put https://github.com/Azure/azure-sdk-tools/pull/12337/files into |
1320fad to
3bacc34
Compare
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 nothing wrong here, but I can't see any changes to eng/common that were made that would go along with this, so I'm taking it on faith somewhat.
eng/common change is here: https://github.com/Azure/azure-sdk-for-rust/pull/3111/files |
So still not using default PowerShell behaviors. As long as those functions correctly catch this case, though, I suppose that'll work. |
|
@danieljurek @hallipr why did you revert this? This is the correct fix over Azure/azure-sdk-tools#12337, we should not be relying on write-error we should be handling exit codes correctly. |
Reverts #3106
Example failure (test and analyze failures expected): https://dev.azure.com/azure-sdk/public/_build/results?buildId=5418072&view=results
Example success: https://dev.azure.com/azure-sdk/public/_build/results?buildId=5418071&view=results