feat: adds the ability to change the default tag name (#27) #28
Conversation
hohmannr
force-pushed
the
feat/swap-tag-name
branch
from
9812e96 to
2b5072f
Compare
Signed-off-by: hohmannr <raphael.hohmann@tuta.io>
hohmannr
force-pushed
the
feat/swap-tag-name
branch
from
2b5072f to
b102547
Compare
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #28 +/- ##
==========================================
+ Coverage 88.95% 89.04% +0.09%
==========================================
Files 12 12
Lines 1747 1762 +15
==========================================
+ Hits 1554 1569 +15
Misses 138 138
Partials 55 55
☔ View full report in Codecov by Sentry. |
|
The idea is great, but not sure how you will handle the maintainability and readability of your application later. |
|
That is a very good question. We have solved this issue with another package, here goes the story: We have lightweight wrappers around multiple packages, which set them up to our needs. E.g. configuring them in an type UserInfo struct {
FirstName string `validate_gateway:"required,max=256" validate_api:"required,max=256"`
LastName string `validate_gateway:"required,max=256" validate_api:"required,max=256"`
Source string `validate_gateway:"eq=gateway" validate_api:"eq=api"`
Timestamp string `validate_gateway:"_rfc3339" validate_api:"_rfc3339"`
}The same strategy can be used for type UserInfo struct {
FirstName string `faker_gateway:"first_name" faker_api:"first_name"`
LastName string `faker_gateway:"last_name" faker_api:"last_name"`
Source string `faker_gateway:"_source_gateway" faker_api:"_source_api"`
Timestamp string `faker_gateway:"_rfc3339" faker_api:"_rfc3339"`
} |
pkg/errors/generic.go
Outdated
| @@ -16,6 +16,7 @@ var ( | |||
| ErrTagNotSupported = "Tag unsupported: %s" | |||
| ErrTagAlreadyExists = "Tag exists" | |||
| ErrTagDoesNotExist = "Tag does not exist" | |||
| ErrTagNameInvalid = "Tag name is invalid" | |||
There was a problem hiding this comment.
Not sure about this error tho.
It brings confusion with the existing error message related to tag -- see other error related to tag
Can we renamed it with something else?
There was a problem hiding this comment.
Curious how validator library decide their error message
There was a problem hiding this comment.
They do not check for valid tagnames^^ https://github.com/go-playground/validator/blob/bd1113d5c1901c5205fc0a7af031a11268ff13ee/validator_instance.go#L154
What I would suppose and what I have seen somewhere else (do not remember where anymore), they called the tag name field tag identifier. This would make for more succinct error names.
ErrFieldTagIdentifierInvalid = "Field tag identifier invalid"Apart from this, one could make it clear for the user of the package, what went wrong, by copying the approach of ErrTagNotSupported and use ErrFieldTagIdentifierInvalid = "Field tag identifier invalid: %s" in conjunction with fmt.Errorf(ErrFieldTagIdentifierInvalid, "<bad-tag-identifier>")
There was a problem hiding this comment.
ErrFieldTagIdentifierInvalid LGTM, can u help to update
pkg/options/options.go
Outdated
| // WithTagName sets the tag name to use. Default tag name is 'faker'. | ||
| func WithTagName(tagName string) OptionFunc { | ||
| if tagName == "" { | ||
| err := errors.New(fakerErrors.ErrTagDoesNotExist) |
There was a problem hiding this comment.
This error may bring confusion, is it they tag key, or the tag name?
There was a problem hiding this comment.
Yes you are right. Do not know what I was thinking. Should be the new error that was introduced in pkg/errors/generic.go
|
Hey @bxcodec! Hope you are fine! Just wanted to quickly check the status on this PR? |
This would solve the problem described in #27.
I have added the ability to change the default tag name to a custom one, which is configurable via an
options.OptionFunc. I tried to be minimal invasive and have added a test to cover the custom tag case and an example file, so people can find this feature in go.pkg reference and the github repo.