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

APIS-3408 Allow Avro to support generation of nullable nested objects #84

Merged
merged 3 commits into from Sep 6, 2019

Conversation

ahornerr
Copy link
Contributor

@ahornerr ahornerr commented Sep 5, 2019

No description provided.

Copy link
Contributor

@jstorer jstorer left a comment

Choose a reason for hiding this comment

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

might need a go mod tidy

Copy link
Contributor

@tkuhlman tkuhlman left a comment

Choose a reason for hiding this comment

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

Looks great, good job figuring it out.
I'll send all AST questions your way now :)

if goFile != nil {
return nil, fmt.Errorf("name %q is not unique in go files at path %q", name, path)
// It's necessary to loop over all the decls and their specs to ensure the typeSpec.Name.Name matches our name
// because the `ast.FilterPackage` doesn't filter out declarations that only have the wanted itemName as a
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting it doesn't filter out when the name is an argument. I bet that was a pain to find.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't too terrible but not immediately obvious what was going on. The ast package doesn't give you the ability to easily filter these out like we need unfortunately.

@ahornerr ahornerr merged commit 6abc556 into master Sep 6, 2019
@ahornerr ahornerr deleted the avro-records-nullable branch September 6, 2019 01:11
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.

None yet

3 participants