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

chore: try removing tox-gh-actions #65

Closed

Conversation

c0state
Copy link
Contributor

@c0state c0state commented Sep 6, 2024

Test if we actually need tox-gh-actions

@c0state c0state changed the base branch from master to chore/migrate_github_actions September 6, 2024 02:17
@@ -32,7 +32,7 @@ jobs:
python -VV
python -m site
python -m pip install --upgrade pip setuptools wheel
python -m pip install --upgrade coverage[toml] tox tox-gh-actions
Copy link
Contributor Author

@c0state c0state Sep 6, 2024

Choose a reason for hiding this comment

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

@matt-mclaughlin-quantum-si , so removing tox-gh-actions also works but it's not as explicit as using it.

The tests basically try to run all tox defined environments but ends up running only the desired one since each worker in the matrix installs a specific Python version (not 100% true since it looks like Python 3.10 is in all the base images).

I'm leaning towards keeping tox-gh-actions since the behavior is more explicit, though it's another dependency. wdyt?

Copy link
Contributor

@matt-m-mclaughlin matt-m-mclaughlin Sep 6, 2024

Choose a reason for hiding this comment

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

@c0state I agree with the premise here in that removing another dependency and keeping the tox setup as boilerplate as possible is ideal. In this setup, each matrix does run the corresponding tests for the pertinent python version, but it also runs the 310base as well each time. I suspect the reason why is that the github runners come pre-installed with python 3.10, which is why the 3.10 env erroneously gets run for all the matrices:

https://github.com/actions/runner-images/blob/ubuntu22/20240901.1/images/ubuntu/Ubuntu2204-Readme.md#language-and-runtime

I think we should either figure out a way to parameterize the matrix runs to only run the pertinent env if we go with this approach. Or we could use the tox-gh-actions, which I believe does exactly that under the hood.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Will close and keep using the plug-in for now

@c0state c0state closed this Sep 6, 2024
@c0state c0state deleted the chore/test_tox_gh_dep_removal branch September 6, 2024 18:37
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.

2 participants