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

Improved typing #10

Merged
merged 4 commits into from
Jun 28, 2021
Merged

Improved typing #10

merged 4 commits into from
Jun 28, 2021

Conversation

aantn
Copy link
Collaborator

@aantn aantn commented Jun 28, 2021

Not much to see for now... I started playing around with some static type checkers and fixing the issues they found. Lets merge this into master soon-ish so the fixes don't have merge conflicts later on.

@aantn aantn requested a review from arikalon1 June 28, 2021 18:50
@aantn
Copy link
Collaborator Author

aantn commented Jun 28, 2021

Just to clarify, I haven't done much testing. I recommend merging after the next release so we have time to find any issues due to messed up imports etc.

@@ -20,7 +20,7 @@ def __init__(self, deploy_func, func, trigger_params, action_params=None):
self.trigger_params = trigger_params
self.action_params = action_params
self.playbook_id = playbook_hash(func, trigger_params, action_params)
if getattr(action_params, "pre_deploy_func", None) is not None:
if action_params is not None and getattr(action_params, "pre_deploy_func", None) is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why we need this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was to make the type checker happy

@arikalon1 arikalon1 merged commit 22ff608 into master Jun 28, 2021
@aantn aantn deleted the improved_typing branch June 29, 2021 05:38
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