fix!: Name field in config does not affect path anymore #265
fix!: Name field in config does not affect path anymore #265amanenk wants to merge 16 commits intocloudquery:mainfrom
Conversation
yevgenypats
left a comment
There was a problem hiding this comment.
Nice catch with the windows bug!
I like that. we just need to update the documentation and also I made it a breaking change.
Agree this will make it less confusing on the connection of name and path as name will be just required alias and path required path of the plugin.
Will let @hermanschaaf take a look as well.
hermanschaaf
left a comment
There was a problem hiding this comment.
Great fixes. I suggested a few small changes.
For releasing this: we should mark this SDK change as breaking (already done), but we should also remember that the CLI update will be breaking. If we are thinking about any other backwards-incompatible config changes, we should bundle them in that release too. (Here I'm thinking about the two spec config entries confusing some users)
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
|
After some discussion, we decided to hold back on this change so as to not force a breaking change so soon after the release of v1. But in the meantime we should start updating documentation to always include an explicit When we have a few more breaking changes to make, we will bundle it together in a new release. |
This updates (and replaces) this earlier PR for making `path` a required config parameter: #265 We have received numerous user reports of the distinction between name and path being confusing, so rather than infer path from name in some circumstances (and not in some others), this change makes things consistent by always requiring both. I've also added a small hint for users running into this error when upgrading from earlier versions. We will not consider this a breaking SDK change, as it doesn't change any protocols, but it will be a breaking CLI change. - relates to #317 - closes #256
Summary
closes #256
Also fixed a bug with multi config files on windows.
Use the following steps to ensure your PR is ready to be reviewed
go fmtto format your code 🖊golangci-lint run🚨 (install golangci-lint here)