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.

Sign up for GitHub

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

Jump to bottom

fix steps panic #783

Merged
merged 5 commits into from
Feb 23, 2023
Merged

fix steps panic #783

merged 5 commits into from
Feb 23, 2023

Conversation

alixander
Copy link
Collaborator

@alixander alixander commented Feb 9, 2023 •
edited
Loading

just added the panicking test. i know it's not valid, but it shouldn't panic

@alixander alixander requested a review from nhooyr February 9, 2023 23:07
nhooyr
nhooyr approved these changes Feb 9, 2023
View reviewed changes
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

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

oh yes definitely should not should be an error. I guess somewhere I'm assuming each child of a steps is a map.

@alixander alixander changed the title todo: panic with steps fix steps panic Feb 22, 2023
@alixander alixander marked this pull request as ready for review February 22, 2023 21:45
@alixander alixander requested a review from nhooyr February 22, 2023 21:45
@alixander
Copy link
Collaborator Author

I guess somewhere I'm assuming each child of a steps is a map.

your hunch is correct

nhooyr
nhooyr suggested changes Feb 22, 2023
View reviewed changes
nhooyr
nhooyr suggested changes Feb 22, 2023
View reviewed changes
@alixander alixander requested a review from nhooyr February 22, 2023 22:59
nhooyr
nhooyr suggested changes Feb 23, 2023
View reviewed changes
@alixander alixander requested a review from nhooyr February 23, 2023 00:21
nhooyr
nhooyr approved these changes Feb 23, 2023
View reviewed changes
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

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

✅

@alixander alixander merged commit a734380 into terrastruct:master Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nhooyr nhooyr nhooyr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alixander @nhooyr