-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 tox.ini #9276
Fix tox.ini #9276
Conversation
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 3668385433
💛 - Coveralls |
@@ -14,7 +14,7 @@ setenv = | |||
QISKIT_SUPRESS_PACKAGING_WARNINGS=Y | |||
QISKIT_TEST_CAPTURE_STREAMS=1 | |||
QISKIT_PARALLEL=FALSE | |||
passenv = RAYON_NUM_THREADS OMP_NUM_THREADS QISKIT_PARALLEL RUST_BACKTRACE SETUPTOOLS_ENABLE_FEATURES QISKIT_TESTS QISKIT_IN_PARALLEL | |||
passenv = RAYON_NUM_THREADS, OMP_NUM_THREADS, QISKIT_PARALLEL, RUST_BACKTRACE, SETUPTOOLS_ENABLE_FEATURES, QISKIT_TESTS, QISKIT_IN_PARALLEL |
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.
There's still whitespace here between the commas and the starts of the next variables. Did you mean RAYON_NUM_THREADS,OMP_NUM_THREADS,...
?
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 thought comma + whitespace is fine like
https://github.com/Qiskit/qiskit-terra/blob/9c1c18cea4b195749a7c9ed7b278f7a32db63b94/tox.ini#L3
Let's see CI's result.
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.
CI all passed, so you're almost certainly right.
(cherry picked from commit e70ce50) # Conflicts: # tox.ini
I came across this while comparing notes on what to do about tox 4 in qiskit-experiments. If you care about still supporting tox 3, it's better to use newline separation based on this comment. The developers don't particularly encourage supporting tox 3 and 4 together though based on the following comment. |
* fix tox.ini (#9276) (cherry picked from commit e70ce50) # Conflicts: # tox.ini * Fix merge conflict * pin ipywidgets (#9272) (cherry picked from commit b4f4c34) Co-authored-by: Takashi Imamichi <[email protected]> Co-authored-by: Jake Lishman <[email protected]> Co-authored-by: Takashi Imamichi <[email protected]>
Summary
Updates tox.ini to avoid the following error due to tox-dev/tox#2671 (tox 4.0.6 released on Dec 11)
log
Details and comments