avro icon indicating copy to clipboard operation
avro copied to clipboard

feat: add enum generator

Open EduardoLaranjo opened this issue 9 months ago • 5 comments

Goal of this PR

This PR tackles issue #428 by updating the avro generator to generate enums with the following format:

const (
	SPADES   Cards = "SPADES"
	HEARTS   Cards = "HEARTS"
	DIAMONDS Cards = "DIAMONDS"
	CLUBS    Cards = "CLUBS"
)

Instead of the just strings

How did I test it?

Regenerate the golden files

EduardoLaranjo avatar Apr 15 '25 22:04 EduardoLaranjo

In order to get this reviewed, please fill in the PR description.

nrwiersma avatar Apr 16 '25 05:04 nrwiersma

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.

EduardoLaranjo avatar Apr 16 '25 09:04 EduardoLaranjo

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

EduardoLaranjo avatar Apr 23 '25 10:04 EduardoLaranjo

I was wondering if you have any idea to avoid name clashing.

Conventionally speaking, it gets prefixed with the type, which would be singular. Like this:

const (
	CardSpades   Card = "SPADES"
	CardHearts   Card = "HEARTS"
	CardDiamonds Card = "DIAMONDS"
	CardClubs    Card = "CLUBS"
)

nrwiersma avatar Apr 23 '25 16:04 nrwiersma

hi @nrwiersma I added the enum generation as optional and add one test

EduardoLaranjo avatar May 12 '25 22:05 EduardoLaranjo

waiting for this to be merged .. thanks @nrwiersma

gdbi avatar Aug 05 '25 19:08 gdbi

Waiting for the review to be addressed.

nrwiersma avatar Aug 06 '25 02:08 nrwiersma