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

Update type hint in add method of StructDef #37

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pmbarrett314
Copy link
Contributor

Since StructDef.add can accept instances of other subclasses of _BaseDef for the datatype, this type hint doesn't cover all possible cases, which annoys my IDE and type checker.

Since StructDef.add can accept instances of other subclasses of _BaseDef for the datatype, this type hint doesn't cover all possible cases, which annoys my IDE and type checker.
@midstar
Copy link
Owner

midstar commented Jul 14, 2022

Hi! Change looks fine for me but please run black (https://pypi.org/project/black/) on your updated files and update your branch with the result. Our CI fails unless all files has been "blacked"

@pmbarrett314
Copy link
Contributor Author

I ran black and committed that. Looks like CI is finding some pylint issues now. I don't think either of these are in my change, though, it is just a single line change in a doc comment.

@midstar
Copy link
Owner

midstar commented Aug 5, 2022

Sorry for the late response. I'm on vacation and don't have access to a computer. I will check it when I'm back.

@pmbarrett314
Copy link
Contributor Author

No worries, it's not exactly an urgent bug. Have a good vacation!

@midstar
Copy link
Owner

midstar commented Aug 13, 2022

Author

The issues on this pull request is due to changed rules in black and pylint. I have fixed this on master branch. Please perform a new pull request against the new master and it will probably be fine :-)

pmbarrett314 added a commit to pmbarrett314/pycstruct that referenced this pull request Sep 8, 2022
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