Skip to content

feat(gen): add ability to pass type metadata #526

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
Merged
nrwiersma merged 1 commit into from
May 9, 2025
Merged

feat(gen): add ability to pass type metadata #526

nrwiersma merged 1 commit into from
May 9, 2025

Conversation

@ljank
Copy link
Contributor

@ljank ljank commented Apr 22, 2025

Goal of this PR

We're looking for ways to pass down some schema metadata to the template, e.g. we have a need for the original schema subject identifier in the generated Go struct.

This change allows to pass arbitrary metadata while parsing the schema and have that metadata available in the template.

How did I test it?

go test

nrwiersma
nrwiersma requested changes Apr 22, 2025
View reviewed changes
@ljank ljank force-pushed the feature/gen/add-metadata branch 3 times, most recently from e7ca557 to 63b6234 Compare April 23, 2025 07:04
@nrwiersma
Copy link
Member

nrwiersma commented Apr 23, 2025

Is there a reason the existing metadata is not enough?

https://github.com/hamba/avro/blob/main/gen/gen.go#L174

@ljank
Copy link
Contributor Author

ljank commented Apr 24, 2025

Unfortunately current metadata is not enough (at least very inconvenient) when it comes to mapping of schema metadata -> typedef metadata. In the template Typedefs have different names compared to schemas:

avro/gen/gen.go

Lines 311 to 316 in 5c80dfb

func (g *Generator) resolveTypeName(s avro.NamedSchema) string {
if g.fullName {
return g.nameCaser.ToPascal(s.FullName())
}
return g.nameCaser.ToPascal(s.Name())
}

Since Typedef names are rendered using unexported nameCaser and unexported resolveTypeName(), we don't know the final Typedef name in advance and therefore can't set corresponding metadata key for access from the template.

Exporting nameCaser and/or resolveTypeName() doesn't feel right, therefore we suggest adding metadata along with the schema when calling (Generator) Parse() :)

@ljank
Copy link
Contributor Author

ljank commented Apr 29, 2025

@nrwiersma what do you think? can we proceed with the proposed change? 🙏🏻

@nrwiersma
Copy link
Member

nrwiersma commented Apr 29, 2025

@ljank I am away at the moment, will take a look when I find some time.

nrwiersma
nrwiersma reviewed May 6, 2025
View reviewed changes
Copy link
Member

@nrwiersma nrwiersma left a comment

Choose a reason for hiding this comment

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

Please fix the linter issues, then I think it is good to go.

@ljank
feat(gen): add ability to pass type metadata
77e7dab 
We're looking for ways to pass down some schema metadata to the template, e.g. we have a need for the original schema subject identifier in the generated Go struct.

This change allows to pass arbitrary metadata while parsing the schema and have that metadata available in the template.
@ljank ljank force-pushed the feature/gen/add-metadata branch from d32371d to 77e7dab Compare May 9, 2025 15:51
@nrwiersma nrwiersma merged commit 13d0f22 into hamba:main May 9, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nrwiersma nrwiersma nrwiersma approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ljank @nrwiersma