-
Notifications
You must be signed in to change notification settings - Fork 120
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
deps: Upgrade open-component-model/ocm to v0.4.0 #1808
Changes from 1 commit
7295bd3
88849ce
4b0a72e
e08fd1f
107e20a
d9be8dd
2436019
b70ce80
9a3fac0
4bbba59
2a9eaa8
111faad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,16 +2,14 @@ package module | |
|
||
import ( | ||
"fmt" | ||
"os" | ||
"strings" | ||
|
||
"github.com/kyma-project/cli/pkg/module/gitsource" | ||
"github.com/mandelsoft/vfs/pkg/projectionfs" | ||
"github.com/mandelsoft/vfs/pkg/vfs" | ||
"github.com/open-component-model/ocm/pkg/common/accessobj" | ||
ocm "github.com/open-component-model/ocm/pkg/contexts/ocm/compdesc" | ||
"github.com/open-component-model/ocm/pkg/contexts/ocm/cpi" | ||
"github.com/open-component-model/ocm/pkg/contexts/ocm/repositories/comparch" | ||
"os" | ||
"sigs.k8s.io/yaml" | ||
) | ||
|
||
|
@@ -61,21 +59,17 @@ func CreateArchive(fs vfs.FileSystem, path string, cd *ocm.ComponentDescriptor) | |
) | ||
} | ||
|
||
// AddSources adds the sources to the component descriptor. If the def.Source is a git repository | ||
func AddSources(cd *ocm.ComponentDescriptor, def *Definition, gitRemote string) error { | ||
if strings.HasSuffix(def.Source, ".git") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This condition was added as part of #1751. Is it still working? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The overall flow of this adding of git source meta information is very illogical and needed some changes. One of these is I made this function explicit, see |
||
gitSource := gitsource.NewGitSource() | ||
var err error | ||
if def.Repo, err = gitSource.DetermineRepositoryURL(gitRemote, def.Repo, def.Source); err != nil { | ||
return err | ||
} | ||
src, err := gitSource.FetchSource(def.Source, def.Repo, def.Version) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
appendSourcesForCd(cd, src) | ||
// AddGitSources adds the git sources to the component descriptor | ||
func AddGitSources(cd *ocm.ComponentDescriptor, def *Definition, gitRemote string) error { | ||
var err error | ||
if def.Repo, err = gitsource.DetermineRepositoryURL(gitRemote, def.Repo, def.Source); err != nil { | ||
return err | ||
} | ||
src, err := gitsource.FetchSource(def.Source, def.Repo, def.Version) | ||
if err != nil { | ||
return err | ||
} | ||
appendSourcesForCd(cd, src) | ||
|
||
return nil | ||
} | ||
|
This file was deleted.
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.
return nil
is not expected, right?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.
It is, the flow just got inverted since this is the last step it can be skipped if there is no reg given
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.
but if no regstry given, this create module command is meaningless, right? It can't push image to remote.
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.
i guess it's like this because the command could also be used for just persisting the archive locally to inspect layers? also the registry flag is also not mandatory which is weird if the command should be meaningless without it. i don't know the intentions/requirements when this command was implemented, but agree that this should have a second look at some point.
and it's not only the registry flow that seems inconsistent: also the generation of the template could be interesting to be used without registry for debugging purposes, but at the moment the flow ends there