feat(fields): read/set all the fields from the given struct#8
feat(fields): read/set all the fields from the given struct#8bkrukowski wants to merge 39 commits intomainfrom
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @bkrukowski - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 3 issues found
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
setter/setter.go
Outdated
| func Set(strct any, field string, val any, convert bool) error { | ||
| return reflect.Set(strct, field, val, convert) | ||
| return intReflect.Set(strct, field, val, convert) | ||
| } |
There was a problem hiding this comment.
suggestion (code_clarification): Consider documenting the CallbackOption type and its intended use.
Public types and functions should have comments explaining their purpose, especially in a library setting. This helps users understand how to use them effectively.
setter/setter.go
Outdated
| func Set(strct any, field string, val any, convert bool) error { | ||
| return reflect.Set(strct, field, val, convert) | ||
| return intReflect.Set(strct, field, val, convert) | ||
| } |
There was a problem hiding this comment.
suggestion (code_clarification): Missing documentation for ConvertType function.
As a public function, ConvertType should have a comment explaining its purpose, parameters, and effects.
setter/setter.go
Outdated
| func Set(strct any, field string, val any, convert bool) error { | ||
| return reflect.Set(strct, field, val, convert) | ||
| return intReflect.Set(strct, field, val, convert) | ||
| } |
There was a problem hiding this comment.
suggestion (code_clarification): Missing documentation for ConvertToPointer function.
It's important to document public functions to guide users on their usage. ConvertToPointer should have a clear description.
setter/setter.go
Outdated
| func Set(strct any, field string, val any, convert bool) error { | ||
| return reflect.Set(strct, field, val, convert) | ||
| return intReflect.Set(strct, field, val, convert) | ||
| } |
There was a problem hiding this comment.
question (code_clarification): Clarify behavior when ok == false in SetByCallback documentation.
The documentation should explicitly state what happens when the callback returns ok == false. Does it skip the field, or is there a default behavior?
setter/setter.go
Outdated
| func Set(strct any, field string, val any, convert bool) error { | ||
| return reflect.Set(strct, field, val, convert) | ||
| return intReflect.Set(strct, field, val, convert) | ||
| } |
There was a problem hiding this comment.
suggestion (code_refinement): Consider making callbackConfig public or providing a public constructor.
Since callbackConfig is used in public API functions, users might need to construct it directly or access its fields.
setter/examples_test.go
Outdated
|
|
||
| func ExampleSetByCallback() { | ||
| var person struct { | ||
| Name string `custom:"name"` |
There was a problem hiding this comment.
suggestion (testing): Consider adding a test case for SetByCallback with conversion options.
The current test case for SetByCallback does not cover scenarios where conversion options (ConvertType and ConvertToPointer) are used. It would be beneficial to add test cases that utilize these options to ensure they work as expected.
setter/examples_test.go
Outdated
|
|
||
| func ExampleSetByCallback() { | ||
| var person struct { | ||
| Name string `custom:"name"` |
There was a problem hiding this comment.
suggestion (testing): Add negative test cases for SetByCallback.
It's important to also test how SetByCallback behaves under error conditions or when the callback returns false. Consider adding tests where the callback intentionally fails to find a matching tag or value in the config map.
setter/examples_test.go
Outdated
|
|
||
| func ExampleSetByCallback() { | ||
| var person struct { | ||
| Name string `custom:"name"` |
There was a problem hiding this comment.
suggestion (testing): Verify the final state of person struct in ExampleSetByCallback.
The test case ExampleSetByCallback sets values on a person struct but does not assert or verify the final state of the struct. It would be useful to add an output comment at the end of the function to verify that the person struct was correctly modified.
Pull Request Test Coverage Report for Build 11133530666Details
💛 - Coveralls |
decorate errors
added `PrefillNilStructs`
added `Path`
added `Path`
replace `[]reflect.StructField` by `fields.Path`
added `ExampleGetter`
added `ExampleConvertToPointers`
use `any`
cleanup
added `ExampleReadJSON`
examples
`CompareNames` => `EqualNames`
bkrukowski
changed the title
setter.SetByCallbackaddlicense
examples
cleanup
tests `TestIterateFields`
tests `TestIterateFields`
use `any`
add argument `v` to: * `ConvertTypes` * `ConvertToPointers` * `Recursive`
cleanup
cleanup
cleanup
* rename `ok` to `set` * added `ExampleBlank`
cleanup
cleanup
refactor
refactor
refactor
refactor
refactor
|
|
Moved to #10 |
2 participants
No description provided.