Skip to content

Conversation

@cchris28
Copy link
Collaborator

per Drew's suggestion.
My first time for github action so there may well be a better way.

@cchris28 cchris28 requested a review from drewoldag April 25, 2023 19:57
Copy link
Collaborator

@drewoldag drewoldag left a comment

Choose a reason for hiding this comment

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

Looks ok. But I do think we're getting into that territory where the number of tests is getting kinda large.

Let's look into tox for local testing

sudo apt-get update
python -m pip install --upgrade pip
python -m pip install copier
python -m pip install copier mypy
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this is necessary. I think that the dependency should be installed when we call pip install .[dev] a couple steps later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I put it in because my trial pull (done in a personal github) failed to find it!
that said, I do not understand WHY it wasn't found. Maybe it was a transient failure?
I've had trouble with github in the last 48 hours with several uses of the template timing out and so it is a bit difficult to untangle.

cd "../test/${{ matrix.copier_config.foldername }}"
mypy src tests
- name: mypy checks strict
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we have a configuration for strict mypy case. Do you think it's worth changing one of the existing configurations so that we are exercising this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We do not! Either we could add one (your bloat concern relevant) or modify one or assume it isn't terribly important.
One thing I also discovered during this exercise was that:
mypy tests fails as I described to you
mypy src tests passes presumably because it knows about src before processing tests. Feature or bug?

@cchris28 cchris28 merged commit c74c67b into main Apr 25, 2023
@cchris28 cchris28 deleted the cchris28/ci-mypy-test branch April 25, 2023 21:20
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.

3 participants