-
Notifications
You must be signed in to change notification settings - Fork 62
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
New command to list the custom packages #466
base: main
Are you sure you want to change the base?
Conversation
cmoulliard
commented
Dec 24, 2024
- New command to list the custom packages
- See: Add list package(s) command #465
Example of what the code produce now Custom packages added
What the command
|
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.
Can we add additional printer columns for the package type?
https://book.kubebuilder.io/reference/generating-crd#additional-printer-columns
https://github.com/cnoe-io/idpbuilder/blob/main/api/v1alpha1/custom_package_types.go
pkg/entity/package.go
Outdated
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.
Can we merge all structs in separate files into one file? Is the entity
package for types? If so, we should name the package as types
instead of entity
.
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.
If so, we should name the package as
types
instead ofentity
.
types is a better name. I will rename. Fixed: 6415f7b
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.
Can we merge all structs in separate files into one file?
Why do you want to merge them into one file ? This is easier to figure out what it exists if you have one file/type under the folder name types
Yes but what do you want to print under this new column ? |
Question: Should we also add the column of the Argocd repo as we also have those for git or gitea repo ?
|
Signed-off-by: cmoulliard <[email protected]>
Signed-off-by: cmoulliard <[email protected]>
…mPackages Signed-off-by: cmoulliard <[email protected]>
Signed-off-by: cmoulliard <[email protected]>
Co-authored-by: Manabu McCloskey <[email protected]> Signed-off-by: cmoulliard <[email protected]>
Co-authored-by: Manabu McCloskey <[email protected]> Signed-off-by: cmoulliard <[email protected]>
Signed-off-by: cmoulliard <[email protected]>
Signed-off-by: cmoulliard <[email protected]>
0cd0d7d
to
cdb89b7
Compare
Signed-off-by: cmoulliard <[email protected]>
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.
Let's merge all files under the types directory. I don't think we should have a file for each struct. It's very Java like right now.😅
pkg/printer/package.go
Outdated
func generatePackageTable(packagesTable []types.Package) metav1.Table { | ||
table := &metav1.Table{} | ||
table.ColumnDefinitions = []metav1.TableColumnDefinition{ | ||
{Name: "Custom package name", Type: "string"}, |
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 liked "name" more. Custom package is already implied when you run the command.
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.
Fixed: 9bbd4d5
You mean ArgoCD application URL or git repositories used by ArgoCD applications? |
Indeed but that simplify the readability of the code and Java language rocks from this point of view :-) |
Argocd URL on our IDP to access the application straight from the Argocd UI |
Signed-off-by: cmoulliard <[email protected]>
Yeah I'd rather keep it uniform within this project. If you look at how we define related types for this project, we do that in a single file like here: https://github.com/cnoe-io/idpbuilder/blob/main/api/v1alpha1/custom_package_types.go So let's merge them into a single file. |
I see, then we should change the name of the column. It's currently named |