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

Release v1.1.3 #54

Merged
merged 4 commits into from
Dec 20, 2019
Merged

Release v1.1.3 #54

merged 4 commits into from
Dec 20, 2019

Conversation

sco1
Copy link
Owner

@sco1 sco1 commented Dec 19, 2019

Fixed

  • Add missing classifier test cases for POSONLYARGS
  • Re-add the tree argument to the checker so flake8 identifies the plugin as needing to run

In the v1.1.2 release, the tree input to the checker was removed in favor of instead requesting the lines of the file to be linted, allowing for a more generic handling of both stdin and file input. Though tree was never used, since the AST from flake8 is untyped, flake8 requires the checker instance to request at least one argument from (physical_line, logical_line, tree) to be present in the checker signature, otherwise the checker will not be run. tree has been readded so we actually lint the files again.

A test would be good to have for this scenario so we don't regress in the future, but doesn't need to block the new release while being worked on.

* Add links to PyPI badges so they link to PyPI instead of the badge image
* Tidy up testing & coverage sections now that we've moved to tox
* Remove pipenv test & coverage scripts in favor of tox
As currently architected, flake8 requires the checker instance to request at least one argument from: `physical_line`, `logical_line`, or `tree`, otherwise the checker will not be run.
Copy link

@SebastiaanZ SebastiaanZ left a comment

Choose a reason for hiding this comment

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

Tested on my system with Python 3.7 and Python 3.8, both in action and with the test suite.

for function in func_list:
if function.name == match_name:
return function
return [function for function in func_list if function.name == match_name][0]
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have just ignored coverage for that if statement because this change is arguably worse than the original. Anyway, this could be improved, and behave more like the old code, bu using next((...), None).

Copy link
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

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

LGTM. Installed and ran it on some unannotated code to test.

@sco1 sco1 merged commit 704fab2 into master Dec 20, 2019
@sco1 sco1 deleted the dev-next branch December 20, 2019 13:06
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