-
Notifications
You must be signed in to change notification settings - Fork 14
Add repository option to crossplane project init
#105
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ import ( | |
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/google/go-containerregistry/pkg/name" | ||
| "k8s.io/apimachinery/pkg/util/validation" | ||
|
|
||
| "github.com/crossplane/crossplane-runtime/v2/pkg/errors" | ||
|
|
@@ -38,8 +39,10 @@ const projectFileName = "crossplane-project.yaml" | |
|
|
||
| // initCmd initializes a new project. | ||
| type initCmd struct { | ||
| Name string `arg:"" help:"The name of the new project."` | ||
| Directory string `help:"Directory to initialize. Defaults to project name." short:"d" type:"path"` | ||
| Name string `arg:"" help:"The name of the new project."` | ||
| Directory string `help:"Directory to initialize. Defaults to project name." short:"d" type:"path"` | ||
| Registry string `default:"example.com/my-org" help:"Override the registry in the project file." optional:"" short:"r"` | ||
| Repository string `help:"Override the repository name in the project file. Defaults to the project name." optional:""` | ||
| } | ||
|
|
||
| func (c *initCmd) Help() string { | ||
|
|
@@ -55,12 +58,24 @@ func (c *initCmd) Run(sp terminal.SpinnerPrinter) error { | |
| if c.Directory == "" { | ||
| c.Directory = c.Name | ||
| } | ||
|
|
||
| if strings.TrimSpace(c.Repository) == "" { | ||
| c.Repository = c.Name | ||
| } | ||
| // Check if the target directory is suitable. | ||
| if err := c.checkTargetDirectory(); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| rp := strings.TrimRight(strings.TrimSpace(c.Registry), "/") | ||
| if rp == "" { | ||
| return errors.New("registry cannot be empty; set --registryPath to an OCI registry prefix like 'xpkg.crossplane.io/my-org'") | ||
| } | ||
|
Comment on lines
+70
to
+72
Contributor
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. Use the actual flag name in the empty-registry error message. Line 71 points users to Suggested patch- return errors.New("registry cannot be empty; set --registryPath to an OCI registry prefix like 'xpkg.crossplane.io/my-org'")
+ return errors.New("registry cannot be empty; set --registry to an OCI repository prefix like 'xpkg.crossplane.io/my-org'")As per coding guidelines, CLI errors in 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| r, err := name.NewRepository(rp + "/" + c.Repository) | ||
| if err != nil { | ||
| return errors.Wrapf(err, "cannot build function reference from repository \"%s/%s\"", rp, c.Repository) | ||
| } | ||
|
Comment on lines
+74
to
+77
Contributor
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. Replace internal “function reference” wording with The wrapped message says “cannot build function reference…”, which is confusing in this command path. Please use user-facing wording tied to invalid Suggested patch- return errors.Wrapf(err, "cannot build function reference from repository \"%s/%s\"", rp, c.Repository)
+ return errors.Wrapf(err, "invalid repository value %q from --registry/--repository", rp+"/"+c.Repository)Based on learnings, concise error strings are preferred when they clearly state the issue; as per coding guidelines for 🤖 Prompt for AI AgentsSources: Coding guidelines, Learnings |
||
|
|
||
| return sp.WrapWithSuccessSpinner("Initializing project", func() error { | ||
| if err := os.MkdirAll(c.Directory, 0o750); err != nil { | ||
| return errors.Wrapf(err, "failed to create directory %s", c.Directory) | ||
|
|
@@ -73,8 +88,8 @@ kind: Project | |
| metadata: | ||
| name: %s | ||
| spec: | ||
| repository: example.com/my-org/%s | ||
| `, c.Name, c.Name) | ||
| repository: %s | ||
| `, c.Name, r.String()) | ||
|
|
||
| if err := os.WriteFile(projFile, []byte(content), 0o600); err != nil { | ||
| return errors.Wrapf(err, "failed to write %s", projectFileName) | ||
|
|
||
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.
Normalize
Repositoryafter trimming, before fallback and validation.Right now
TrimSpaceis only used for the emptiness check, but Line 74 still consumes the untrimmed value. Could we persist the trimmed value so accidental leading/trailing spaces don’t cause avoidable parse failures?Suggested patch
🤖 Prompt for AI Agents