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.

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

refactor: check for types in custom name extraction #73

Merged
merged 8 commits into from
Feb 1, 2022
Merged

refactor: check for types in custom name extraction #73

merged 8 commits into from
Feb 1, 2022

Conversation

iqdecay
Copy link
Contributor

@iqdecay iqdecay commented Jan 28, 2022 •
edited

This PR refactors code to check that the type we convert to is the one declared in ABI custom type generation.
This closes issue #61.
EDIT: built on top of #67 which should be merged first.

@iqdecay iqdecay self-assigned this Jan 28, 2022
@iqdecay iqdecay linked an issue Jan 28, 2022 that may be closed by this pull request
ABIgen custom types do not check that the declared type matches the concrete type #61
Closed
@iqdecay iqdecay marked this pull request as draft January 28, 2022 18:55
@iqdecay iqdecay added the enhancement New feature or request label Jan 28, 2022
@iqdecay iqdecay marked this pull request as ready for review January 31, 2022 05:04
digorithm
digorithm requested changes Jan 31, 2022
View reviewed changes
Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

It's looking really good, really appreciate all those tests! Left a couple of comments.

fuels-core/src/code_gen/custom_types_gen.rs Outdated Show resolved Hide resolved
fuels-core/src/code_gen/custom_types_gen.rs Outdated Show resolved Hide resolved
fuels-core/src/code_gen/functions_gen.rs Outdated Show resolved Hide resolved
fuels-core/src/code_gen/functions_gen.rs Outdated Show resolved Hide resolved
fuels-core/src/code_gen/functions_gen.rs Show resolved Hide resolved
fuels-core/src/code_gen/functions_gen.rs Show resolved Hide resolved
fuels-core/src/code_gen/functions_gen.rs Show resolved Hide resolved
@digorithm digorithm mentioned this pull request Jan 31, 2022
Closed
@iqdecay
Copy link
Contributor Author

iqdecay commented Jan 31, 2022

Normally done w/ all changes!

digorithm
digorithm requested changes Jan 31, 2022
View reviewed changes
fuels-core/src/code_gen/functions_gen.rs Outdated Show resolved Hide resolved
fuels-core/src/code_gen/functions_gen.rs Outdated Show resolved Hide resolved
fuels-core/src/code_gen/functions_gen.rs Outdated Show resolved Hide resolved
digorithm
digorithm approved these changes Feb 1, 2022
View reviewed changes
Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

Great job, looking much cleaner, and love the tests!

@iqdecay iqdecay merged commit 6bd67a5 into master Feb 1, 2022
@iqdecay iqdecay deleted the vnepveu/refactor-abigen-types branch February 1, 2022 20:11
@iqdecay iqdecay mentioned this pull request Feb 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@digorithm digorithm digorithm approved these changes

Assignees

@iqdecay iqdecay

Labels
enhancement New feature or request
Projects
Fuel Network
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ABIgen custom types do not check that the declared type matches the concrete type
2 participants
@iqdecay @digorithm