-
Notifications
You must be signed in to change notification settings - Fork 34
Fix compatibility with tox>=4 #163
base: master
Are you sure you want to change the base?
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.
Makes sense to me, but I similarly cannot test it or debug it.
@ryanhiebert I can make a mock project using cookiecutter-pypackage, which features tox-travis integration, and then attempt to link it to a new Travis CI account and hopefully I'll be able to see if the changes work properly. |
So I have some good news, after making a new travis account with a boilerplate cookiecutter project, which includes my build of tox-travis, the (Gitlab) project has successfully built on Travis CI, and tox tests completed successfully. Results are here: https://app.travis-ci.com/gitlab/ZenulAbidin/Ragnarok Ideally let's get this merged before my Travis trial credit expires again :) |
@ryanhiebert any update on this? |
@tox.hookimpl | ||
def tox_addoption(parser): | ||
@impl | ||
def tox_add_option(parser): |
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.
Do we know when this was added to tox? Do we need to restrict the lower bound?
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.
Likely you should restrict to latest release, because that's also against what you're running the CI.
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.
See my comment below - this is not compatible with tox<4.
Are we talking about restricting the `tox` version at requirements.txt, or somewhere else?
…On Mon, May 15, 2023 at 8:47 PM, Bernát Gábor ***@***.***> wrote:
@gaborbernat commented on this pull request.
---------------------------------------------------------------
In [src/tox_travis/hooks.py](#163 (comment)):
> @@ -15,8 +16,8 @@
from .after import travis_after
***@***.***
-def tox_addoption(parser):
***@***.***
+def tox_add_option(parser):
Likely you should restrict to latest release, because that's also against what you're running the CI.
—
Reply to this email directly, [view it on GitHub](#163 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AKE46J73XOZKMBLYOIMWFCDXGJT4BANCNFSM6AAAAAAX6AE6NM).
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
In the |
The change appears to have kicked in starting from tox-dev/tox#1991 (Changed Files), which was packaged in version 4.0.0a7. Looks like it really is tox>=4-only material. Also there is this thing at the bottom of the Changelog page of Tox:
Perhaps we should make a branch of the existing codebase with a name like tox-v3 or something that is only for older tox versions. |
This commit introduces compatibility with Tox version 4 by making the following changes:
_split_envs
function is surgically pasted from old Tox code to v4_workarounds.@impl
decorator and not@tox.hookimpl
.add_option
, while the configure hook is switched to tox_env_config (I wasn't 100% sure doing this, but it seems to make tox4 run successfully, so let me know if there is a more correct way to port it.)If you need Tox4 support immediately and can't wait for this to get merged, put this in your
requirements.txt
:I have not been able to test this live on Travis CI, because builds have been completely broken for me since recently and customer support is non-responsive.