-
Notifications
You must be signed in to change notification settings - Fork 12
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
add UnitUpgradeStep, improve add_step #219
add UnitUpgradeStep, improve add_step #219
Conversation
Add UnitUpgradeStep and is_unit_upgrade_step, also use sub_steps as property to prevent defining it as not list of BaseStep's. Raising TypeError in add_step if object is not based on BaseStep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM. I have some small comments that are non-blockers
It's not totally clear to me the impact of those changes in the big picture, but I can see that the introduction of unit upgrade step might be necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some docstring changes suggested in line. Otherwise looks good.
Add UnitUpgradeStep and is_unit_upgrade_step, also use sub_steps as property to prevent defining it as not list of BaseStep's. Raising TypeError in add_step if object is not based on BaseStep.
Add UnitUpgradeStep and is_unit_upgrade_step, also use sub_steps as property to prevent defining it as not list of BaseStep's. Raising TypeError in add_step if object is not based on BaseStep.