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

Check enum values before pass to native side. #145

Merged
merged 5 commits into from
Oct 30, 2017

Conversation

hwanseung
Copy link
Contributor

Check enum values before pass to native side.

ISSUE=#80

@hwanseung hwanseung requested a review from romandev as a code owner October 22, 2017 07:15
@hwanseung
Copy link
Contributor Author

@romandev
this PR is same with initial #124.

@hwanseung
Copy link
Contributor Author

i rebased this patch.

@romandev
Copy link
Member

@hwanseung Your patch is broken in bots :(

@hwanseung
Copy link
Contributor Author

@romandev i fixed it.

Copy link
Member

@romandev romandev left a comment

Choose a reason for hiding this comment

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

Looks good overall.
I think we don't need to generate enum_types_cpp(.h, .cc).

#include <set>
#include <string>

class EnumTypes {
Copy link
Member

Choose a reason for hiding this comment

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

IMHO, we don't need to generate this type.
We can just pass the string set into isValidEnum() directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you means to pass the string set into isVaildEnum() directly in interface_cpp.njk?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, exactly!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i changed to pass to the string set as parameter.

env.renderString(enum_type_cpp_tmpl, {enums: idl_enum}));
}

// TODO(hwansueng) :: this function should be improved.
Copy link
Member

Choose a reason for hiding this comment

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

nit: // TODO(hwanseung): This function should be improved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@romandev romandev merged commit 75f2507 into lunchclass:master Oct 30, 2017
@hwanseung hwanseung deleted the auto_gen_enum branch November 2, 2017 01:38
byeolbit pushed a commit to byeolbit/bacardi that referenced this pull request Nov 16, 2017
Check enum values before pass to native side.

ISSUE=lunchclass#80
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.

2 participants