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

Check pip upgrade return code #460

Merged
merged 2 commits into from
Oct 19, 2023

Conversation

patkenneally
Copy link
Contributor

@patkenneally patkenneally commented Oct 18, 2023

Description

On line 310 in conanfile.py the Basilisk directory is pip installed. This line interprets any output to stderr (returned in the err variable) as an error. However, characters in stderr don't necessarily mean an error (it's sometimes simply a warning). Rather, this line should call process.returncode and check for a success 0, or error code 1 or otherwise.

Verification

CI will run successfully.

Documentation

None needed, none invalidated.

Future work

The follow on work that gave rise to this change is coming in #459.

Characters in stderr don't necessarily mean an error. Rather,
a call to process.returncode should check for a success 0,
or error code 1 (or otherwise).
@patkenneally patkenneally force-pushed the ci/bsk-458--check-pip-process-returncode branch from 96c6a19 to 7f19f55 Compare October 18, 2023 22:50
@schaubh schaubh force-pushed the ci/bsk-458--check-pip-process-returncode branch from 7f19f55 to db0f615 Compare October 19, 2023 00:36
Copy link
Contributor

@schaubh schaubh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed some small RST formatting issue. I updated the BSK release notes on this branch already. Good to go.

@patkenneally patkenneally merged commit 4c26e17 into develop Oct 19, 2023
2 checks passed
@patkenneally patkenneally deleted the ci/bsk-458--check-pip-process-returncode branch October 19, 2023 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants