Skip to content
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

Nonzero Exit Code for performance-setup Failures #59483

Merged
merged 6 commits into from
Oct 5, 2021

Conversation

Tohron
Copy link
Contributor

@Tohron Tohron commented Sep 22, 2021

Created to address dotnet/performance#2010
This PR aims to cause performance-setup.ps1 and performance-setup.sh to return a nonzero exit code whenever a command in the script fails.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@Tohron Tohron added the tenet-performance Performance related issue label Sep 22, 2021
@Tohron Tohron self-assigned this Sep 22, 2021
@Tohron Tohron requested a review from billwert September 22, 2021 17:54
@Tohron Tohron marked this pull request as draft September 22, 2021 17:55
@DrewScoggins
Copy link
Member

@Tohron Since we have so much repeated code in these scripts from the error checking, I think that it would be useful to write a function that takes in the command we want to check, and then does the error level checking. This would greatly simplify the code, and make adding new checks much easier.

@Tohron Tohron requested a review from billwert September 30, 2021 20:16
@Tohron Tohron marked this pull request as ready for review September 30, 2021 22:04
@billwert billwert merged commit 191c60e into main Oct 5, 2021
@akoeplinger akoeplinger deleted the performance-setup-error-check branch October 7, 2021 18:37
@ghost ghost locked as resolved and limited conversation to collaborators Nov 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tenet-performance Performance related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants