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 support for structures and enumerations. #310

Merged
merged 13 commits into from
Sep 15, 2021
Merged

Add support for structures and enumerations. #310

merged 13 commits into from
Sep 15, 2021

Conversation

KlimMixer
Copy link
Contributor

Fixes: #266

Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

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

Thanks @KlimMixer!

@KlimMixer
Copy link
Contributor Author

@frangio Thank you. Updated, and fixed all problems.

@frangio
Copy link
Contributor

frangio commented Jul 14, 2021

I was wrong about documentation, these nodes don't have documentation in the AST. I also decided to simplify things a little, enum and struct members don't need to be Linkable. Please take a look and let me know if you have any feedback.

@KlimMixer
Copy link
Contributor Author

Thank you for the simplification and nice realization of differentiation between functions events modifiers and other contract items.
And next, I will write tests and push for the review.

@frangio
Copy link
Contributor

frangio commented Jul 15, 2021

For testing you should make sure there are structs and enums in one of the fixtures like fixtures/basic for example.

@frangio
Copy link
Contributor

frangio commented Jul 20, 2021

Can you create a single new fixture with both struct and enum? And give it a custom template (see how custom does it) that specifically tests struct and enum and its members.

frangio
frangio previously approved these changes Sep 15, 2021
Copy link
Contributor

@frangio frangio 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. Thank oyu @KlimMixer!

@frangio frangio enabled auto-merge (squash) September 15, 2021 22:34
@frangio frangio merged commit 59e19ba into OpenZeppelin:master Sep 15, 2021
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.

Docs don't include structs
2 participants