feat: add enum generator #524
Conversation
|
In order to get this reviewed, please fill in the PR description. |
|
Apologies @nrwiersma, I have update the PR description. I understand you are doing some refactor on generator, so this PR might not be needed, let me know. |
nrwiersma
left a comment
nrwiersma
left a comment
There was a problem hiding this comment.
Nice change. I would be concerned that this will break some peoples code. IMO, it would be better to put it behind a flag/option to avoid this and maintain the status quo.
Yep I can make as an option. I was wondering if you have any idea to avoid name clashing. I will also create a dedicated test for this feature |
Conventionally speaking, it gets prefixed with the type, which would be singular. Like this: |
EduardoLaranjo
force-pushed
the
main
branch
2 times, most recently
from
4ccd9d2 to
c1f4590
Compare
|
hi @nrwiersma I added the enum generation as optional and add one test |
gen/testdata/golden.go
Outdated
| type Cards string | ||
|
|
||
| const ( | ||
| SPADES Cards = "SPADES" |
There was a problem hiding this comment.
This should not be all upper. It should also be prefixed with the type. So CardSpades.
gen/testdata/golden.go
Outdated
| "github.com/hamba/avro/v2" | ||
| ) | ||
|
|
||
| type Cards string |
There was a problem hiding this comment.
This should be singular. It does not represent multiple cards, but a card.
|
waiting for this to be merged .. thanks @nrwiersma |
|
Waiting for the review to be addressed. |
3 participants
Goal of this PR
This PR tackles issue #428 by updating the avro generator to generate enums with the following format:
Instead of the just strings
How did I test it?
Regenerate the golden files