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

Combine ci workflows with tox #289

Merged
merged 3 commits into from
Sep 11, 2020
Merged

Combine ci workflows with tox #289

merged 3 commits into from
Sep 11, 2020

Conversation

jenhagg
Copy link
Collaborator

@jenhagg jenhagg commented Sep 10, 2020

Purpose

I started this earlier and thought it might be useful to finish. The goal is to move the workflow to the tox.ini file rather than the github .yml so both local and ci usages will be able to use it. As part of doing that, I added isort since it's described in the wiki but we haven't been using it regularly, and this should make it easy.

What it does

First define tox.ini which allows either checking formatting (and running tests) with no side effects, or actually formatting the code locally before committing changes. Then combine these into the github workflow and delete the obsolete .yml. The pytest and formatting steps show up as separate checkmarks due to the strategy block in the .yml. This failed ci as expected, so I ran tox -e format which calls isort (in a way that is compatible with black), and is responsible for most of the changes.

To use it locally, running tox will default to running the pytest-local and format environments, which will in turn run the matching commands. Specifically, it will match the lines starting with pytest and local, as well as the formatting ones. Environments (called "testenvs" in tox language) can be invoked specifically, e.g. tox -e checkformatting would validate formatting and output a diff if it fails.

Note, this is meant to be totally optional, and running any of these as we do now will still work, the only change is that we would use tox in our ci pipeline.

Time to review

15 min - this isn't super important, I just thought I'd put this out if people are interested.

@jenhagg jenhagg force-pushed the jon/tox branch 2 times, most recently from 1cd6361 to 6fa4772 Compare September 11, 2020 00:33
Copy link
Collaborator

@rouille rouille left a comment

Choose a reason for hiding this comment

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

It works well. Thanks

@jenhagg jenhagg merged commit 40009a7 into develop Sep 11, 2020
@jenhagg jenhagg deleted the jon/tox branch September 11, 2020 17:58
@ahurli ahurli mentioned this pull request Mar 11, 2021
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