expose labels Application Creation Option to cli #271
Conversation
fallback to global git config fixed a bunch of tests
Codecov Report
@@ Coverage Diff @@
## main #271 +/- ##
==========================================
- Coverage 61.67% 61.15% -0.52%
==========================================
Files 17 17
Lines 2894 2984 +90
==========================================
+ Hits 1785 1825 +40
- Misses 1016 1060 +44
- Partials 93 99 +6
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
| @@ -114,6 +114,7 @@ func AddFlags(cmd *cobra.Command) *CreateOptions { | |||
| cmd.Flags().StringVar(&opts.DestNamespace, "dest-namespace", "", "K8s target namespace (overrides the namespace specified in the kustomization.yaml)") | |||
| cmd.Flags().StringVar(&opts.InstallationMode, "installation-mode", InstallationModeNormal, "One of: normal|flat. "+ | |||
| "If flat, will commit the application manifests (after running kustomize build), otherwise will commit the kustomization.yaml") | |||
| cmd.Flags().StringToStringVar(&opts.Labels, "labels", nil, "Optional labels that will be set on the Application resource. (e.g. \"{{ placeholder }}=my-org\"") | |||
There was a problem hiding this comment.
Not sure I understand the example here, the (e.g. \"{{ placeholder }}=my-org\" part. Which probably means no one will understand it. We need to think of a better way to explain or to just drop the explanation here and leave it for the docs.
There was a problem hiding this comment.
yeah, you are right. i'll add a special md file to document how to use labels in project create and then in app create.
| @@ -63,9 +64,11 @@ type ( | |||
| url string | |||
There was a problem hiding this comment.
lets separate the public and private props with one blank line, just so it's easier to understand.
pkg/git/repository.go
Outdated
| var h plumbing.Hash | ||
|
|
||
| cfg, err := r.ConfigScoped(config.SystemScope) | ||
| author, err := r.getAuthor(ctx, opts.Provider) |
There was a problem hiding this comment.
passing opts.Provider here does nothing since you already have your provider instance on opts.provider
pkg/git/repository.go
Outdated
| func (r *repo) getAuthor(ctx context.Context, providerType string) (*object.Signature, error) { | ||
| username, email, err := r.provider.GetAuthor(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to get commiter data. Error: %w", err) |
There was a problem hiding this comment.
commiter -> committer
There was a problem hiding this comment.
Usually you should not do: fmt.Errorf("something. Error: %w") you just do fmt.Errorf("something: %w", err).
There was a problem hiding this comment.
Should we fail to commit if we don't get the author from the provider? Does it make sense the a provider will not return an error but will have an empty username/email?
There was a problem hiding this comment.
currently, github/gitlab/gitea should all return valid username/email from the token (or fail with an error).
but in ado, it just returns "", "", nil, and then the user will get the message about missing information in git config file, because this will be the only way they can get it to work.
pkg/git/repository.go
Outdated
| } | ||
|
|
||
| if username == "" || email == "" { | ||
| return nil, fmt.Errorf("failed to get author data. Please make sure your gitconfig contains a name and an email") |
There was a problem hiding this comment.
"failed to get author data" -> "missing required author information in git config, make sure your git config contains a 'user.name' and 'user.email'"
|
|
||
| if cfg.User.Name == "" || cfg.User.Email == "" { | ||
| return nil, fmt.Errorf("failed to commit. Please make sure your gitconfig contains a name and an email") | ||
| return nil, err |
There was a problem hiding this comment.
fmt.Errorf("failed to get author information: %w", err)
also - extract git commit author information from token (for github/gitlab/gitea). fallback to using global gitconfig data in azure devops.