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.

Sign up for GitHub

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

Jump to bottom

Fold in knative.dev/client-pkg as a submodule #1953

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Fold in knative.dev/client-pkg as a submodule #1953

wants to merge 12 commits into from

Conversation

cardil
Copy link
Contributor

@cardil cardil commented Jun 20, 2024 •
edited

Description

This PR merges code from the knative.dev/client-pkg, that is missing in this repo, onto the knative.dev/client/pkg. TBD a PR to deprecate the whole of the knative.dev/client-pkg.

Changes

  • 🧹 This PR merges code from the knative.dev/client-pkg, that is missing in this repo, onto the knative.dev/client/pkg
  • 🧹 Publicize the code in lib/ dir onto pkg/
  • 🧹 Remove the extra kn sub-package
  • 🧹 Makes pkg/ as a Go submodule and introduces Go's workspace.

Reference

Fixes #1941
Closes #1935
Closes knative/client-pkg#177
Requires knative/hack#386

Release Note

The knative.dev/client-pkg package is now deprecated in favor of the knative.dev/client/pkg package.

@knative-prow Knative Prow
Copy link

knative-prow bot commented Jun 20, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 20, 2024
knative-prow[bot]
knative-prow bot reviewed Jun 20, 2024
View reviewed changes
Copy link

@knative-prow knative-prow bot left a comment

Choose a reason for hiding this comment

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

@cardil: 1 warning.

In response to this:

Description

This PR merges code from the knative.dev/client-pkg, that is missing in this repo, onto the knative.dev/client/pkg. TBD a PR to deprecate the whole of the knative.dev/client-pkg.

Changes

  • 🧹 This PR merges code from the knative.dev/client-pkg, that is missing in this repo, onto the knative.dev/client/pkg
  • 🧹 Publicize the code in lib/ dir onto pkg/
  • 🧹 Remove the extra kn sub-package
  • 🧹 Makes pkg/ as a Go submodule and introduces Go's workspace.

Reference

Fixes #1941

Release Note

The [knative.dev/client-pkg](https://pkg.go.dev/knative.dev/client-pkg) is now deprecated in favor of the [knative.dev/client/pkg](https://pkg.go.dev/knative.dev/client/pkg) module.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

hack/tools.go Show resolved Hide resolved
@knative-prow knative-prow bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 20, 2024
@dsimansk
Copy link
Contributor

/test all

@codecov Codecov
Copy link

codecov bot commented Jun 25, 2024 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 30.61%. Comparing base (ccd3fea) to head (7804812).

Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #1953       +/-   ##
===========================================
- Coverage   76.82%   30.61%   -46.22%     
===========================================
  Files         207        6      -201     
  Lines       12892      245    -12647     
===========================================
- Hits         9904       75     -9829     
+ Misses       2187      164     -2023     
+ Partials      801        6      -795

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cardil
Copy link
Contributor Author

cardil commented Jul 2, 2024

/test all

cardil
cardil commented Jul 2, 2024
View reviewed changes
go.mod Outdated
require k8s.io/utils v0.0.0-20240102154912-e7106e64919e
replace knative.dev/client/pkg => ./pkg

replace knative.dev/hack => github.com/cardil/knative-hack v0.0.0-20240702091202-6535957027e0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

/hold

Requires knative/hack#386

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 2, 2024
@cardil
Copy link
Contributor Author

cardil commented Jul 9, 2024

/test all

@cardil cardil mentioned this pull request Jul 9, 2024
Merged
@cardil
Copy link
Contributor Author

cardil commented Jul 10, 2024

/test build-tests

@cardil
Copy link
Contributor Author

cardil commented Jul 10, 2024

/test unit-tests

@psschwei psschwei removed their request for review July 16, 2024 12:48
@cardil
Copy link
Contributor Author

cardil commented Jul 18, 2024

/test all

@cardil
Copy link
Contributor Author

cardil commented Jul 18, 2024

/test all

@cardil cardil mentioned this pull request Jul 19, 2024
Open
@cardil
Copy link
Contributor Author

cardil commented Jul 19, 2024

The integration test fails because of net-istio bug: knative-extensions/net-istio#1345

@cardil
Copy link
Contributor Author

cardil commented Jul 19, 2024

/test all

@cardil
Copy link
Contributor Author

cardil commented Jul 19, 2024

/test all

@cardil
Copy link
Contributor Author

cardil commented Jul 19, 2024

/test all

@cardil
Copy link
Contributor Author

cardil commented Jul 19, 2024

/test all

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 19, 2024
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 21, 2024
@knative-prow Knative Prow
Copy link

knative-prow bot commented Aug 21, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cardil
Once this PR has been reviewed and has the lgtm label, please assign rhuss for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knative-prow Knative Prow knative-prow left review comments

@itsmurugappan itsmurugappan Awaiting requested review from itsmurugappan

@rhuss rhuss Awaiting requested review from rhuss

Assignees
No one assigned
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Proposal: Fold back in client-pkg to Go submodule
3 participants
@cardil @dsimansk @knative-prow-robot