Skip to content

Release v1.1.1#40

Merged
sco1 merged 34 commits into
masterfrom
v1.1.1-dev
Dec 8, 2019
Merged

Release v1.1.1#40
sco1 merged 34 commits into
masterfrom
v1.1.1-dev

Conversation

@sco1
Copy link
Copy Markdown
Owner

@sco1 sco1 commented Sep 26, 2019

PR for v1.1.1 release

Added

  • Add pipenv-setup as a dev dependency & CI check to ensure synchronization between Pipfile and setup.py
  • Add tox configuration for local testing across Python versions
  • Add test for checking a single yield of TYP301 per function
  • Add coverage reporting to test suite
  • Add testing for positional only arguments

Changed

  • typed_ast is now required only for Python versions < 3.8
  • Update flake8 minimum version to 3.7.9 for Python 3.8 compatibility
  • Refactor Tests #50 Completely refactor test suite for maintainability

Fixed

  • Fix mixed type hint tests not being run due to misnamed test class
  • Fix TYP301 classification issue where error is not yielded if the first argument is type annotated and the remaining arguments have type comments

Closes: #44
Closes: #45
Closes: #46
Closes: #43

@sco1 sco1 added this to the v1.1.1 milestone Sep 26, 2019
@sco1
Copy link
Copy Markdown
Owner Author

sco1 commented Oct 31, 2019

Opening this up for review.

Testing passes locally & Python 3.8 support in CI will be here soonTM.

@sco1 sco1 marked this pull request as ready for review October 31, 2019 19:01
@sco1 sco1 changed the title v1.1.1 Release Placeholder Release v1.1.1 Oct 31, 2019
We're probably ok allowing for minor versions of flake8 to be installed. Since `~=` isn't recognized by `pipenv-setup` we have to define this as a range instead.
@sco1
Copy link
Copy Markdown
Owner Author

sco1 commented Nov 8, 2019

3.8 is now (unofficially) present on Azure 🎉

sco1 added 6 commits November 20, 2019 19:13
Splitting the test parameters across multiple files makes test specification fairly fragile and difficult to both maintain and add additional cases to.

This refactor migrates test case source & truth parameters into a single file, allowing test cases to be specified in one place and in discrete chunks, removing the risk that adjusting one test case negatively impacts testing on the unchanged test cases.

The test fixtures have also been adjusted to accommodate the change in test parameter layout.
So I don't completely miss tests again

And because it's a super good idea
* Fix classifier logic only flagging mixed type hints if the type comment is first. This was due to only using a sentinel flag for the type comment instead of both the comment & annotation
* Add test case for the type annotation coming before the type comment
* Modify mixed type hint test to check that TYP301 is only being yielded once per function & add a test case
* Rename type comment test to better describe what is being tested
sco1 added 2 commits November 21, 2019 18:17
* Fix TYP301 classification issue where error is not yielded if the first argument is type annotated and the remaining arguments have type comments
* Fix misnamed mixed type test so it actually runs
* Add test for single TYP301 yield per function
@sco1
Copy link
Copy Markdown
Owner Author

sco1 commented Nov 22, 2019

I've merged in some of the refactoring being undertaken as part of #50 to fix some bugs that were identified on this release branch. The summary in the OP has been updated.

@sco1 sco1 mentioned this pull request Nov 22, 2019
8 tasks
Comment thread flake8_annotations/checker.py Outdated
Copy link
Copy Markdown

@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.

Looks good to me.

I've read the source code a few times, ran the test suite locally (although I don't have P3.6, so that was skipped), and did some manual testing as well. I do want to note that I'm a bit out of my comfort zone reviewing this.

Comment thread testing/helpers.py
@sco1 sco1 merged commit 961819a into master Dec 8, 2019
@sco1 sco1 deleted the v1.1.1-dev branch December 8, 2019 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants