Skip to content
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

feat: Add metadata and status helpers to Kyma type #2179

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

c-pius
Copy link
Contributor

@c-pius c-pius commented Jan 16, 2025

Description

Changes proposed in this pull request:

-adds helper functions to the Kyma type

Related issue(s)

@c-pius c-pius requested a review from a team as a code owner January 16, 2025 10:07
@kyma-bot kyma-bot added cla: yes Indicates the PR's author has signed the CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 16, 2025
@c-pius c-pius linked an issue Jan 16, 2025 that may be closed by this pull request
7 tasks
@c-pius c-pius force-pushed the feat/add-kyma-helpers branch 2 times, most recently from 3f06f47 to 49690d4 Compare January 17, 2025 08:10
@lindnerby lindnerby self-assigned this Jan 17, 2025
@c-pius c-pius force-pushed the feat/add-kyma-helpers branch 2 times, most recently from c6d35b6 to c440351 Compare January 17, 2025 12:20
@c-pius c-pius force-pushed the feat/add-kyma-helpers branch from c440351 to 67e281c Compare January 17, 2025 13:08
@@ -0,0 +1,208 @@
package v1beta2_test
Copy link
Member

Choose a reason for hiding this comment

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

Can we add this pkg to the coverage file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am afraid this can't be done easily. The api module is not included in our unittests make target as we only list the package from lifecycle-manager. If you don't mind, I would propose to tackle this in a follow-up as we will require some chnages to support that I think.

.PHONY: unittest
unittest: ## Run the unit test suite.
	go test `go list ./... | grep -v /tests/` -coverprofile cover.out -coverpkg=./...

@@ -8,6 +8,8 @@ require (
sigs.k8s.io/controller-runtime v0.19.4
)

require github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

Any idea were these indirect dep changes are coming from?

Copy link
Contributor Author

@c-pius c-pius Jan 17, 2025

Choose a reason for hiding this comment

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

From adding testify/assert.

Copy link
Member

@lindnerby lindnerby left a comment

Choose a reason for hiding this comment

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

Commented for some clarifications 🙌

api/go.mod Outdated
@@ -31,6 +33,7 @@ require (
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/stretchr/testify v1.10.0
Copy link
Member

Choose a reason for hiding this comment

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

Why is testify in the block of indirect dependencies?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates the PR's author has signed the CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maintenance Windows] Skip module upgrades during maintenance window
3 participants