Conversation
Signed-off-by: Nic Cope <nicc@rk0n.org>
|
@phisco @jbw976 @bobh66 @turkenh - Could you prioritize reviewing this please. Mostly because if it looks good I'd like to get crossplane/crossplane#6553 merged before I suffer too many merge conflicts. :) |
| type Unstructured struct { | ||
| unstructured.Unstructured | ||
|
|
||
| Schema Schema |
There was a problem hiding this comment.
So, if I got this correctly, the default schema is modern, for people upgrading that would mean that if they don't explicitly set the legacy one everything will be broken in a not so obvious way. Would it make sense to switch the two?
There was a problem hiding this comment.
Hrm good point. I think I feel okay with this breaking change though. I imagine there's very few users of this package in practice, apart from c/c. The Go SDK for example has its own version. So I doubt anyone will break in practice.
phisco
left a comment
phisco
left a comment
There was a problem hiding this comment.
one question about the breaking change, otherwise LGTM 🙏
2 participants
Description of your changes
Fixes crossplane/crossplane#6505
Closes #844
This is my attempt to address crossplane/crossplane#6505 without removing external secret store support completely - yet (see #842).
I have:
earthly +reviewableto ensure this PR is ready for review.Linked a PR or a docs tracking issue to document this change.Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.