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

Changing Availability to Enum #238

Merged
merged 18 commits into from
Jun 18, 2020
Merged

Conversation

carlosDigio
Copy link
Contributor

Hi @nickrandolph . We have implemented the issue #235 in this PR. Can you check it out?
Thanks

Copy link
Contributor

@nickrandolph nickrandolph left a comment

Choose a reason for hiding this comment

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

I'm not sure we need to define Availability enum in all three languages (ie dart, kt, swift) since we need to serialise it via the channel anyhow. Wouldn't it be easier to keep the use of string for availability in kt/swift and then just decode to an enum in dart?

@carlosDigio
Copy link
Contributor Author

I'm not sure we need to define Availability enum in all three languages (ie dart, kt, swift) since we need to serialise it via the channel anyhow. Wouldn't it be easier to keep the use of string for availability in kt/swift and then just decode to an enum in dart?

Hi! It may not be entirely necessary in this case, but we think that, just as in dart seems correct, in native it would also be more appropriate to use enums rather than strings despite having to do a decoding for use with dart or any other language.

nickrandolph
nickrandolph previously approved these changes Jun 18, 2020
@nickrandolph
Copy link
Contributor

@carlosDigio for some reason the Android build is failing - can you see if you can resolve this please

@nickrandolph nickrandolph changed the title Issue 235 Changing Availability to Enum Jun 18, 2020
@alvaroDigio
Copy link
Contributor

@nickrandolph fixed the build error

@nickrandolph nickrandolph merged commit 1001ba8 into builttoroam:develop Jun 18, 2020
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.

4 participants