fix: Spec unmarshalling now supports defaults and validation #181
Conversation
yevgenypats
force-pushed
the
fix/spec_unmarshal
branch
from
31cde61 to
c87bf0e
Compare
Also, this enable to detect unusued keys which is useful in typos and/or deprecated options.
yevgenypats
force-pushed
the
fix/spec_unmarshal
branch
from
c87bf0e to
7636ec7
Compare
|
Closes cloudquery/cloudquery#2054 |
| "invalid_kind", | ||
| `kind: nice`, | ||
| "failed to decode spec: unknown kind nice", | ||
| nil, |
There was a problem hiding this comment.
nit: let's add keys to these struct instantiations
There was a problem hiding this comment.
I feel for this test driven use-case it reduces the boilerplate imo and makes it easier to add new tests. just opinion ofc.
There was a problem hiding this comment.
Makes sense, though as someone reading it for the first time, I found it hard to know what all these parameters were for, and had to scroll up and down a few times. I think it improves readability if you have the keys right there
| return fmt.Errorf("version is required") | ||
| } | ||
| if !strings.HasPrefix(s.Version, "v") { | ||
| return fmt.Errorf("version must start with v") |
There was a problem hiding this comment.
Do we not support latest version at all anymore?
There was a problem hiding this comment.
nope. per cloudquery/cloudquery#2054 . I think it's the right approach such as with any other package managers (go, npm) they are not supporting latest
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
🤖 I have created a release *beep* *boop* --- ## [0.9.2](v0.9.1...v0.9.2) (2022-09-27) ### Bug Fixes * Spec unmarshalling now supports defaults and validation ([#181](#181)) ([ba9128a](ba9128a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
| if d.Version == "" { | ||
| return fmt.Errorf("version is required") | ||
| } | ||
| if !strings.HasPrefix(d.Version, "v") { |
There was a problem hiding this comment.
Nit, we should do full SemVer validation as the following string vv1.0.0 passes this validation
| return fmt.Errorf("version is required") | ||
| } | ||
| if !strings.HasPrefix(d.Version, "v") { | ||
| return fmt.Errorf("version must start with v") |
There was a problem hiding this comment.
Then change this error message to version must be a valid SemVer string)
Fixes: defaults, validation and unknown keys