Skip to content

Update to C++17 and use std::optional for getSuperType #4203

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
Merged
merged 9 commits into from
Oct 18, 2021
Merged

Update to C++17 and use std::optional for getSuperType #4203

merged 9 commits into from
Oct 18, 2021

Conversation

dschuff
Copy link
Member

@dschuff dschuff commented Oct 2, 2021

No description provided.

tlively
tlively reviewed Oct 2, 2021
View reviewed changes
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

🎉 🎉 🎉

It would be good to move the variable declaration into the if in these cases and the other cases where applicable. LGTM with that!

@dschuff
Copy link
Member Author

dschuff commented Oct 6, 2021

OK, I think this works now. Apparently I picked a bad time to mess with supertypes 🤣

tlively
tlively approved these changes Oct 7, 2021
View reviewed changes
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, and sorry for the recent churn with the supertypes!

@tlively tlively mentioned this pull request Oct 7, 2021
Closed
@dschuff
Copy link
Member Author

dschuff commented Oct 7, 2021

This depends on https://chromium-review.googlesource.com/c/emscripten-releases/+/3195182

@dschuff dschuff enabled auto-merge (squash) October 18, 2021 23:52
@dschuff dschuff merged commit be02d3f into main Oct 18, 2021
@dschuff dschuff deleted the stdoptional branch October 18, 2021 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tlively tlively tlively 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
@dschuff @tlively