-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resource conversion, one solo-kit.json per project #214
Conversation
Issues linked to changelog: |
if err != nil { | ||
return err | ||
} | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we doing this copy? there's a resources.Clone function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The copy needs to happen in place onto dst
-- clone returns a new object
if err != nil { | ||
return err | ||
} | ||
err = protoutils.UnmarshalBytes(srcBytes, dst) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
if err := protoutils.UnmarshalBytes(srcBytes, dst); err != nil {
return err
}
return true | ||
for _, skp := range soloKitProjects { | ||
for _, vc := range skp.ApiGroup.VersionConfigs { | ||
if strings.HasPrefix(protoFile, filepath.Dir(skp.ProjectFile)+"/"+vc.Version) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this means solo-kit.json
is now expected to live (always) in the parent directory of the version dirs? if so, it might be useful to provide a log line warning or error if the user somehow misses this point
for _, vc := range skp.ApiGroup.VersionConfigs { | ||
vc.CustomResources = append(vc.CustomResources, importedResources...) | ||
for _, desc := range descriptors { | ||
if filepath.Dir(desc.ProtoFilePath) == filepath.Dir(skp.ProjectFile)+"/"+vc.Version { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like we have this check in a few places. can we extract to a function?
also, use filepath.Join
here. even though it's only required on windows. and if you use windows, you're wrong 😏
WIP: Need to do some more cleanup and validation.
This PR adds support for converting resources from one version to another, and redefines a "project" from the de facto "one solo-kit.json file per version" to the preferred "one solo-kit.json file per set of versioned api groups". The latter is a breaking change.
BOT NOTES:
resolves #215