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

🌱 Remove dependency on cli module and bump dependencies in cli and sdk #3146

Merged
merged 5 commits into from
Aug 7, 2024

Conversation

embik
Copy link
Member

@embik embik commented Jul 31, 2024

Summary

Since kcp 0.24.0, the new cli package is creating a bit of a problem because its a dependency of github.com/kcp-dev/kcp, but most users would not import it. Thus, the version from go.mod is used for dependency resolution, which is an invalid one:

kcp/go.mod

Line 17 in f4297bd

github.com/kcp-dev/kcp/cli v0.0.0-00010101000000-000000000000

To avoid having a replace directive in every project that imports github.com/kcp-dev/kcp, I've moved the small API surface used in github.com/kcp-dev/kcp (the main module) to github.com/kcp-dev/kcp/sdk, a module that both modules depend on anyway.

This still means that to build a project from github.com/kcp-dev/kcp, you must have github.com/kcp-dev/kcp/sdk as a dependency as well, but that seems to be the more likely case.

In addition, this PR bumps the cli and sdk modules to be in sync with the changes done to the main module in #3140.

After that, I also ran into the issue that we are using a KCP apigen from v0.21.0 (so quite an old one), so I've changed that logic to use the apigen that is present in the repository right now. It doesn't make much sense to me to keep using this from previous releases.

Related issue(s)

Fixes #

Release Notes

Remove the need to put a `replace` directive in place for `github.com/kcp-dev/kcp/cli` when importing `github.com/kcp-dev/kcp`

…ckage

Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
@kcp-ci-bot kcp-ci-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the DCO. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jul 31, 2024
@embik embik requested review from mjudeikis and xrstf July 31, 2024 09:30
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
@kcp-ci-bot kcp-ci-bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Jul 31, 2024
@embik embik changed the title 🌱 Remove dependency on cli module and bump dependencies in cli and sdk WIP: 🌱 Remove dependency on cli module and bump dependencies in cli and sdk Jul 31, 2024
@kcp-ci-bot kcp-ci-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 31, 2024
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
@embik embik changed the title WIP: 🌱 Remove dependency on cli module and bump dependencies in cli and sdk 🌱 Remove dependency on cli module and bump dependencies in cli and sdk Jul 31, 2024
@kcp-ci-bot kcp-ci-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 31, 2024
@embik embik requested a review from sttts August 1, 2024 09:58
@xrstf
Copy link
Contributor

xrstf commented Aug 1, 2024

/approve

@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xrstf

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2024
@mjudeikis
Copy link
Contributor

/lgtm
looks leggit :)

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 7, 2024
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: ee5207b3faf532400592a478441c5f9137ba9248

@embik
Copy link
Member Author

embik commented Aug 7, 2024

/retest

looks like a flake.

@kcp-ci-bot kcp-ci-bot merged commit c4d49aa into kcp-dev:main Aug 7, 2024
19 checks passed
@embik embik deleted the k8s-update-sdk branch August 7, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants