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

Rename the comment tag to e.g. enum:"..." #25

Open
orenl opened this issue Aug 19, 2024 · 2 comments
Open

Rename the comment tag to e.g. enum:"..." #25

orenl opened this issue Aug 19, 2024 · 2 comments
Labels
wontfix This will not be worked on

Comments

@orenl
Copy link
Contributor

orenl commented Aug 19, 2024

The use of json:"..." as a tag in the comment is too generic. It may conflict with casual comments elsewhere in the code (in const or var). Consider renaming to enum:"...".

Note that once #24 get fixed, such conflicts should not happen, since this would be confined to the designated type; Nevertheless, IMO enum:"..." would still be more precise choice.

For example:

package main

type Enum uint8

//go:generate go-enum-encoding -type=Enum
const (
    Plain Enum = iota+1 // json:"one_plain"
)

var (
    Variable int // json:"something"
)
@nikolaydubina
Copy link
Owner

json

json is intentional. we are relying on the same tag that people are already familiar with.
this is natural choice as it follows notation how people are declaring field names for JSON for structs. this reduces cognitive load and allows easy learning. this choice also emphasises, that our concern here is encoding/decoding, and nothing else, which makes limited scope of this tool, which is also beneficial. in fact, I would rather raise proposal to official Go language to include this as official Go language notation.

conflicts with other json tags

best I know, json comment tags can not be used in const nor var declaration. the only place standard Go (and any 3rd party packages I know) are using json tags so far are in struct declaration. this AST tool scans json tags only for const and var declarations. thus no conflicts should happen.

@nikolaydubina nikolaydubina added the wontfix This will not be worked on label Aug 19, 2024
@nikolaydubina
Copy link
Owner

issue is open to collect feedback from others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants