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

#124: Corrected type hint for recipe param "_model" #128

Merged
merged 2 commits into from
Nov 23, 2020

Conversation

GitRon
Copy link
Contributor

@GitRon GitRon commented Nov 13, 2020

@berinhard Here is a PR for the issue :)

@GitRon
Copy link
Contributor Author

GitRon commented Nov 13, 2020

Any idea why the pipeline is not working for me? 🤔

@berinhard
Copy link
Member

@GitRon it's just because you didn't introduce any udpates in the changelog. But the code looks good.

I'm not a heavy user of python's static typing annotation so I'm not familiar with errors or warnings from PyCharm. So, how do you validate the types? Do you use a command line lib or a helper Python module to check the types?

I'm asking because maybe this is something to add in our CI integration process. If it makes sense to you, can you help us with that by updating our Github's action workflow? Maybe we can create a new one call check_type_hints.yml to test and check the type annotations.

@GitRon
Copy link
Contributor Author

GitRon commented Nov 16, 2020

@berinhard PyCharm checks the type of the object (all django models derive from models.Model) to the type hinting. Pretty straight forward. If that's it, what you are asking 😃

@GitRon
Copy link
Contributor Author

GitRon commented Nov 23, 2020

@berinhard Any chance that this gets merged soon? 😃 I'd like to cross it off my list 😎

@berinhard berinhard merged commit 69d110c into model-bakers:main Nov 23, 2020
@GitRon GitRon deleted the bugfix/#124-recipe-wrong-typehint branch November 23, 2020 13:43
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