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

Ensure that warnings are treated as errors and fix encoding errors #94

Merged
merged 4 commits into from
Jul 17, 2023

Conversation

kkirsche
Copy link
Contributor

@kkirsche kkirsche commented Jul 14, 2023

Sorry, I had meant to originally create this as a draft / against my fork, but it ended up here.

This merge request is a follow-up to #93 which fixes:

  1. That warning are treated as errors
  2. That the encoding errors identified in the tests are fixed.

This merge request opens encoding as "utf8" in the mypy visitor to align with the default encoding of all python files as of Python 3. Since the logfile is controlled by bump-pydantic, I used utf8 as well for consistency with the python language files.

@kkirsche kkirsche changed the title Ensure that warnings are treated as errors Ensure that warnings are treated as errors and fix encoding errors Jul 14, 2023
@kkirsche
Copy link
Contributor Author

I'll need to come back to this and install hatch as this seems to introduce errors from workflow dependencies and I can't see from the actions where the encoding warning is truly being generated from in the test, so want to see if it's related to running the app itself (so will install and run it that way)

@kkirsche kkirsche marked this pull request as draft July 14, 2023 13:20
pyproject.toml Show resolved Hide resolved
@Kludex Kludex force-pushed the fix/encoding-tests-fail-on-warnings branch from 27220be to d039a21 Compare July 17, 2023 16:59
@Kludex Kludex marked this pull request as ready for review July 17, 2023 16:59
Copy link
Member

@Kludex Kludex left a comment

Choose a reason for hiding this comment

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

Thanks @kkirsche :)

@Kludex Kludex merged commit f081901 into pydantic:main Jul 17, 2023
@kkirsche
Copy link
Contributor Author

Thanks for your help getting it over the finish line. I appreciate that. Excited to look around and see if any other things may be good opportunities for contributions.

I'm working through transitioning five or six decent size applications at work, so seeing what types of things may be good fits, e.g., is it reasonable to detect a missing dependency on pydantic-settings if you're using BaseSettings in PydanticV1 but don't have the corresponding dependency for V2 specified.

Thanks again, though, and hopefully, we can run into each other on more PRs

@kkirsche kkirsche deleted the fix/encoding-tests-fail-on-warnings branch July 17, 2023 20:52
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