-
Notifications
You must be signed in to change notification settings - Fork 64
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
Feat: Ignore Columns #49
Conversation
Codecov Report
@@ Coverage Diff @@
## main #49 +/- ##
==========================================
+ Coverage 81.14% 82.03% +0.88%
==========================================
Files 14 14
Lines 748 768 +20
==========================================
+ Hits 607 630 +23
+ Misses 120 118 -2
+ Partials 21 20 -1
Continue to review full report at Codecov.
|
Please fix deepsource issues |
model.go
Outdated
@@ -72,6 +72,11 @@ func (t *tagMetaRegistry) Register(table interface{}, opts ...tableMetaOption) ( | |||
for i := 0; i < lens; i++ { | |||
structField := v.Field(i) | |||
tag := structField.Tag.Get("eql") | |||
isIgnore := strings.Contains(tag, "-") |
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.
please help to refactor this part. I think using strings.Contains
is not a good practice
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 will parse the tag by bit operation. All tags are parsed at once and saved by bit operations.
model.go
Outdated
for _, field := range fieldNames { | ||
//has field in the TableMeta | ||
if _, ok := meta.fieldMap[field]; ok { | ||
exist := false |
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 think we don't need this flag, just always `delete(meta.fieldMap, field). And you need to check if this is primary key, I think we'd better not allow users to remove primary key.
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.
ok.
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.
Sorry for late reply because I was busy this month
model.go
Outdated
|
||
// The tag is parsed by bit operation. | ||
// This is better than using strings.Contains queries once for each tag. | ||
func parseTag(tag string) tagBitFlags { |
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.
hhh, you don't need this. Just iterate the tags and the modify the column meta directly
} | ||
|
||
type TestIgnoreModel struct { | ||
Id int64 `eql:"auto_increment,primary_key,-"` |
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 prefer that symbol -
has highest priority. So if we find -
, we just ignore it.
model.go
Outdated
for _, field := range fieldNames { | ||
//has field in the TableMeta | ||
if columnMeta, ok := meta.fieldMap[field]; ok { | ||
if columnMeta.isPrimaryKey { |
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.
don't need to check this. Users should be responsible for their action. They can find they remove the PK when they run tests.
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.
yes.
Please add example for this. I have added examples for some public functions and you can take them as reference |
ok |
ignore columns by Tag and Option.