-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactoring #28
Refactoring #28
Conversation
7ee2abe
to
7e4c83d
Compare
schemas/core/resource.go
Outdated
|
||
// ResourceTyper is the interface implemented by types that can hold Common resource attribute and are can return it ResourceType | ||
type ResourceTyper interface { | ||
ResourceType() *ResourceType | ||
GetCommon() *Common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not simply Common()
?
Do you mean if I refactor ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot use Common()
because its name will collide with Common
struct member.
For the same reason we have GetSchema()
instead of Schema()
:(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right ... Hmmm :\
I propose to rename Common
struct field into CommonAttributes
or something similar.
And then rename GetCommon()
to simply Common()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, so I'm going to fix right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done bf4e508 !
This PR aims to fix #9 and #10.
Also, it makes the naming convention more clear and closer to Golang one.
Eg, now you can use
datatype.New()
instead ofcore.NewDataType()
Notice that, I've not promoted
schemas/attribute
to package because IMHO it's not needed.Maybe, I will add just few commits trying to remove a bit of code dup (eg.
AttrNameExpr
).However, review it ASAP because it's blocking, please.