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

#DI-95 implement tox for vestapol tests #9

Merged
merged 12 commits into from
May 9, 2022
Merged

#DI-95 implement tox for vestapol tests #9

merged 12 commits into from
May 9, 2022

Conversation

EvavW
Copy link
Contributor

@EvavW EvavW commented May 2, 2022

Ticket: #DI-95
Run tests in a reliable environment with tox.
Notes:

  • some small changes were made in the codebase based on feedback from flake8 which now runs as part of tox testing
  • please add comments with suggestions for which versions of Python we should be testing with tox. keep in mind all developers will have to have these installed
  • please add comments with your opinion: should we add pylintor mypy to the tox tests? would require some overhead in terms of configuration and updates to the codebase

@EvavW EvavW changed the title [#DI-95](https://inquirer.atlassian.net/browse/DI-110?atlOrigin=eyJpIjoiNmRjODUxYTUxNzMwNDY4OGE5OTA0Y2ZkMDQ4MjM1NWIiLCJwIjoiaiJ9 implement tox for vestapol tests) #DI-95 implement tox for vestapol tests May 2, 2022
@EvavW EvavW requested review from waligob and ajonesINQ May 2, 2022 16:41
@waligob
Copy link
Collaborator

waligob commented May 4, 2022

Something I was wondering: if poetry is creating a virtual env for you when you run poetry install, how can I select this environment in VSCode? This was apparently the most requested feature in VSCode's GitHub repo, and rolled out last May, but I couldn't get it to work. So instead I ran poetry config virtualenvs.in-project true, which then creates a .venv in my project's root directory, and VSCode automatically detects and activates it.

Not sure if you found a more straightforward way of getting VSCode and poetry environments to play nice.

pyproject.toml Show resolved Hide resolved
vestapol/__init__.py Show resolved Hide resolved
@EvavW
Copy link
Contributor Author

EvavW commented May 4, 2022

@waligob re:poetry virtualenv with vscode

will add this to the README. the solution here worked for me: microsoft/vscode-python#8372 (comment) - you can then select the poetry env from the list of python interpreters

@waligob
Copy link
Collaborator

waligob commented May 4, 2022

the solution here worked for me: microsoft/vscode-python#8372 (comment) - you can then select the poetry env from the list of python interpreters

Nice! Worked for me too. Thanks.

tox.ini Outdated Show resolved Hide resolved
@waligob
Copy link
Collaborator

waligob commented May 5, 2022

please add comments with your opinion: should we add pylint or mypy to the tox tests? would require some overhead in terms of configuration and updates to the codebase

I think adding a linter would be really valuable for producing consistent, readable, and reliable code. I don't have strong opinions on our choice of linter—mypy's static type checking is intriguing given that there are at least a few implicit type constraints in the code (e.g. the json writers). We're using flake8 in our tox.ini—would that also be in contention for our linter solution?

At any rate, given the overhead in implementing the linter, I'd suggest that we create a new ticket for it.

@EvavW
Copy link
Contributor Author

EvavW commented May 5, 2022

please add comments with your opinion: should we add pylint or mypy to the tox tests? would require some overhead in terms of configuration and updates to the codebase

I think adding a linter would be really valuable for producing consistent, readable, and reliable code. I don't have strong opinions on our choice of linter—mypy's static type checking is intriguing given that there are at least a few implicit type constraints in the code (e.g. the json writers). We're using flake8 in our tox.ini—would that also be in contention for our linter solution?

At any rate, given the overhead in implementing the linter, I'd suggest that we create a new ticket for it.

I'm leaning toward mypy as well. I'm most interested in the type checking so I wouldn't really consider flake8 as a contender, and I don't think it will compete too much with mypy. Agree that it should be a separate ticket (really 2 tickets for both vestapol and inq-data-resources).

@EvavW EvavW merged commit 2e63b6a into main May 9, 2022
@EvavW EvavW deleted the tox branch May 9, 2022 20:55
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