Skip to content

Issue 349 buildpack and clusterbuildpack ref#352

Merged
chenbh merged 6 commits intobuildpacks-community:mainfrom
mailprak:issue-349-buildpack-ref
Nov 3, 2023
Merged

Issue 349 buildpack and clusterbuildpack ref#352
chenbh merged 6 commits intobuildpacks-community:mainfrom
mailprak:issue-349-buildpack-ref

Conversation

@mailprak
Copy link
Contributor

@mailprak mailprak commented Sep 5, 2023

Issue : #349

@mailprak mailprak requested a review from a team as a code owner September 5, 2023 17:01
@chenbh
Copy link
Contributor

chenbh commented Sep 7, 2023

@mailprak #349 is referring to the fact that the .spec.order.group of a Builder can refer to a [Cluster]Buildpack object reference (eg {"name": "my-buildpack","kind":"ClusterBuildpack"}).

We need to display the [Cluster]Buildpacks that are actually used by the Builder instead of every single one on the cluster.

@chenbh
Copy link
Contributor

chenbh commented Sep 7, 2023

I forgot to mention it in the original issue, but do you mind also doing the same thing for kp clusterbuilder status as well? The only difference is that the ClusterBuilder will only have ClusterBuildpack refs

@mailprak
Copy link
Contributor Author

I forgot to mention it in the original issue, but do you mind also doing the same thing for kp clusterbuilder status as well? The only difference is that the ClusterBuilder will only have ClusterBuildpack refs

sure

Display buildpack and clusterbuildpack in the status for builder and clusterbuilder
removes unused test
@mailprak
Copy link
Contributor Author

@chenbh have made the changes. Please review.

return err
}

cpTableWriter, err := commands.NewTableWriter(writer, "BuildpackName", " BuildpackKind")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the column headers should be more readable Buildpack Name, Buildpack Kind

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines +142 to +152
for _, entry := range bldr.Spec.Order {
for _, ref := range entry.Group {
if ref.ObjectReference.Name != "" && ref.ObjectReference.Kind != "" {
err := cpTableWriter.AddRow(ref.ObjectReference.Name, ref.ObjectReference.Kind)
if err != nil {
return err
}
}
}
cpTableWriter.Write()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you run gofmt on your files? I think there's some weird whitespace padding in here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return err
}

cpTableWriter, err := commands.NewTableWriter(writer, "ClusterBuildpackName", " ClusterBuildpackKind")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: spaces in column names

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@chenbh
Copy link
Contributor

chenbh commented Nov 2, 2023

Sorry for the delay, added some comments

Updates header names and formatting the file
Update Test cases for header name
@chenbh chenbh linked an issue Nov 3, 2023 that may be closed by this pull request
Copy link
Contributor

@chenbh chenbh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@chenbh chenbh merged commit 8b1ca08 into buildpacks-community:main Nov 3, 2023
neil-hickey pushed a commit to neil-hickey/kpack-cli that referenced this pull request Jan 13, 2026
…uildpack-ref

Issue 349 buildpack and clusterbuildpack ref
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kp builder status should show Buildpack and ClusterBuildpack Refs

2 participants