Skip to content

Conversation

dan-blanchard
Copy link

This PR is a rebased version of #1242 that also:

  • Adds a couple tests
  • Fixes some flipped logic in the original PR where it had if type(arg) is None: instead of if arg is type(None):

@dan-blanchard
Copy link
Author

I've rebased this since it got a little stale

@vitalik
Copy link
Owner

vitalik commented Sep 21, 2025

Hi @dan-blanchard - could you add missing tests coverage on

ninja/signature/details.py lines - 212-215, 355-368

@vitalik
Copy link
Owner

vitalik commented Sep 21, 2025

Note: #1483 can be related

@dan-blanchard dan-blanchard force-pushed the feat/add-uniontype-check branch from f5694c0 to 63d426b Compare October 6, 2025 21:17
if self.streaming:
self.content = b"".join(http_response.streaming_content) # type: ignore
else:
self.content = http_response.content # type: ignore[union-attr]
Copy link
Author

Choose a reason for hiding this comment

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

mypy was complaining about this ignore being unnecessary for me

@dan-blanchard
Copy link
Author

I rebased this on master, added more tests, fixed some bugs that those tests revealed, and then got coverage to 100%.

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