Fix or ignore all warnings #55
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I noticed a warning being issued by this line of code during testing, so I'm fixing the line that writes out a file to make the encoding explicit. This achieves the goal that no code in this project itself issues any warnings.
|
Yep, the idea of this project is people will be able to run the old So if |
diazona
force-pushed
the
fix-warnings-in-test/1/dev
branch
from
14804a5 to
cf0935b
Compare
A warning about the deprecation of distutils in Python 3.12 appears in CI when running under Python 3.11 with old versions of our dependencies. (Most likely it's related to using old setuptools, but I can't reproduce it locally so I can't be sure.) This warning isn't relevant to us because we can import the vendored version of distutils from setuptools itself, so I'm setting the warning filter up to ignore it.
diazona
force-pushed
the
fix-warnings-in-test/1/dev
branch
from
cf0935b to
8fc58ac
Compare
These warnings are issued outside of our code base. I believe they're mostly related to outdated setuptools functionality appearing in our tests (plus the one random warning from pyparsing). But since we have to handle old configuration directives - in fact that's the whole point of the package - there's nothing that makes sense to do about the warnings other than ignoring them.
diazona
force-pushed
the
fix-warnings-in-test/1/dev
branch
from
8fc58ac to
e91713b
Compare
|
This one looks ready. I got rid of all the warnings I saw showing up in CI, not because it was really necessary to do so, but I figured I might as well get them all as long as I was looking into warnings in the first place. We could configure pytest to upgrade all (unignored) warnings to errors if we want to, although it certainly doesn't have to happen as part of this PR. |
sjlongland
approved these changes
Aug 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I fixed the source of one warning related to the code in this project, and ignored some others that are issued by setuptools/distutils code. I think ignoring is the right call there because the whole purpose of this project is to process legacy configuration so we are naturally going to use some deprecated features, but I wouldn't mind having that double-checked.