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

Add ContainerViewController init with AnyWorkflowConvertable #1092

Merged
merged 1 commit into from
Apr 14, 2020

Conversation

dhavalshreyas
Copy link
Contributor

Currently, we don't have an API to create a ContainerViewController with AnyWorkflow type. This adds an extension to make that possible.

This is mostly a copy-paste from code @bencochran shared with me privately.

@dhavalshreyas dhavalshreyas force-pushed the dhaval/containerVCWAnyWorkflow branch from c33468f to 5f7ae42 Compare April 14, 2020 20:18
@JustinDSN
Copy link
Contributor

Should we add a unit test to make sure this works as expected, and stays working?

@bencochran
Copy link
Collaborator

Should we add a unit test to make sure this works as expected, and stays working?

Yeah, that’d probably be a good idea, especially since this relies on Swift knowing that the call to self.init(workflow:… is the other (more-specifically-constrained) initializer and not a recursive call to self.

@dhavalshreyas dhavalshreyas force-pushed the dhaval/containerVCWAnyWorkflow branch from 5f7ae42 to f01e81b Compare April 14, 2020 21:07
@dhavalshreyas
Copy link
Contributor Author

Should we add a unit test to make sure this works as expected, and stays working?

Done!

@bencochran
Copy link
Collaborator

Test looks good, thanks!

@dhavalshreyas dhavalshreyas merged commit 2e198d6 into master Apr 14, 2020
@dhavalshreyas dhavalshreyas deleted the dhaval/containerVCWAnyWorkflow branch April 14, 2020 21:44
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.

3 participants