eaigner / hood

Database agnostic ORM for Go
MIT License
708 stars 52 forks source link

Follow standard convention for struct tags #1

Closed jedsmith closed 11 years ago

jedsmith commented 11 years ago

Hey,

I haven't glanced at the code yet, but based on the examples I'd like to highlight the established convention for struct tags. hood shouldn't claim the entire struct tag, and should coexist peacefully with other things that might be interested (such as encoding/json).

There's an API in reflect, StructTag.Get, which will parse the standard format and give you only the requested section back. So, in all the places that you extract the entire StructTag from the struct using reflect, you can instead use StructTag.Get to parse out a "hood" section with the same behavior, which would allow coexistence like so:

type User struct {
    Name string `json:"user_name,omitempty" hood:"pk"`
}

Very ideal for declaring options for both encoding/json and hood, which I would foresee as a common occurrence.

eaigner commented 11 years ago

I know.

Originally I was using the StructTag.Get method but thought that it was unnecessarily verbose. However, as you pointed out it should be able to coexist with other tags, which is of course a valid point.

I'll change it back so the tags are compatible with StructTag.Get.

jedsmith commented 11 years ago

Cool. I also like in the Hacker News discussion the suggestion to establish sql, if you're careful about forward planning (interoperability with as-yet-unwritten libraries might be useful).

Not trying to poop on the parade. Genuinely happy you wrote this. Just don't want this to become a victim of backward compatibility if it sticks too long, so hoping you can commit to the "right way" now. I don't think the struct tag format is ideal at all, but the documentation has established a precedent, and we should conform as closely as possible to see if the gophers have a grand future vision that we cannot see yet.

(I'm with you. It's unnecessarily verbose and an odd convention on the language, but many things in Go are odd.)

Thanks for quick response, and looking forward to trying this out for my next project. I was in the market, so good timing, and great work.

eaigner commented 11 years ago

I definitely like the "sql:" proposal

eaigner commented 11 years ago

Will change it to sql:"size(128),default('abc')" to match the JSON tag as closely as possible, as proposed in the HN thread.