-
Notifications
You must be signed in to change notification settings - Fork 42
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
Fix tests and address #36 #37
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.
Why did you give up with GA?
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.
Anyway, the tests work fine and the change are definitely an improvement, thanks for taking care of this!
After some discussions with @bernardopacini we decided it was best to do this on Azure to reduce code duplication. |
Purpose
This PR does two things:
subprocess.run
instead ofos.system
Type of change
What types of change is it?
Select the appropriate type(s) that describe this PR
Testing
Existing tests pass.
Checklist
Put an
x
in the boxes that apply.flake8
andblack
to make sure the code adheres to PEP-8 and is consistently formatted