-
Notifications
You must be signed in to change notification settings - Fork 151
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
Replace Tox with Nox #861
Replace Tox with Nox #861
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.
Some minor details we need to handle before merging this. But overall, it shouldn't be too hard
Pull Request Test Coverage Report for Build 8470603505Details
💛 - Coveralls |
60f462e
to
1ac8aa8
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.
Sorry for the super slow review on this, I'm super keen to get us off of tox. I took a quick look through the config and it looks reasonable to me and this looks great, and is much nicer than what we had with tox. I still need to checkout the branch locally and play with it to see how it works in practice for me.
The one thing I think we should avoid doing for this commit is to keep the tox.ini file around for a little bit. Just because we've been using it since the creation of retworkx so leaving the config around for a while and maybe adding python -c print("The use of tox for running tests is no longer supported in a future version of rustworkx you should use nox instead. Refer to the contributing guide for more details on it's usage")
to the tox jobs. I'm just worried about the occasional contributor that comes back after 5 months and tries to run tox and gets frustrated.
I'll delete the |
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 pushed up a small commit to add back the tox.ini file in it's previous form and add the warning to the end of the ouptut. I think it's better to not actively break developer's existing workflows, but just make it explicit we're not supporting it anymore. The cost of keeping the tox.ini file around in the short to medium term is minimal.
I am thinking about wrap the current workflows around nox them i.e. make tox only install nox and then call |
Closes #852
We replace Tox with Nox, overall it seems promising. I do think customizing Nox seems much, much easier given that we have the full power of Python. In fact, I feel we could profit and replace many items in
tools/
too with Nox in the future