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

Drop dynamic wrapper injection code generation #2742

Merged
merged 2 commits into from
May 18, 2023

Conversation

dprotaso
Copy link
Member

Part of #2739

This removes the codegen logic that generates informers and clients that use the dynamic.Client underneath. These aren't being used anywhere.

@knative-prow knative-prow bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 17, 2023
@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 17, 2023
@codecov
Copy link

codecov bot commented May 17, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.03 ⚠️

Comparison is base (300df43) 81.53% compared to head (ca50e75) 81.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2742      +/-   ##
==========================================
- Coverage   81.53%   81.50%   -0.03%     
==========================================
  Files         163      163              
  Lines       10018    10018              
==========================================
- Hits         8168     8165       -3     
- Misses       1606     1608       +2     
- Partials      244      245       +1     
Impacted Files Coverage Δ
apis/duck/v1/source_types.go 48.64% <ø> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dprotaso
Copy link
Member Author

/assign @pierDipi

Copy link
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 18, 2023
@knative-prow
Copy link

knative-prow bot commented May 18, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, pierDipi

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

@knative-prow knative-prow bot merged commit dfb4bf0 into knative:main May 18, 2023
37 checks passed
@dprotaso dprotaso deleted the drop-injection-dynamic-codegen branch May 18, 2023 14:22
creydr added a commit to creydr/knative-eventing-istio that referenced this pull request Jun 19, 2023
…ent_expansion.go

Codegen support for informers and clients that use the dynamic.Client interface
was dropped in knative/pkg#2742.
knative-prow bot pushed a commit to knative-extensions/eventing-istio that referenced this pull request Jun 21, 2023
* Update dependencies to latest

* Remove dependency to knative.dev/pkg/client/injection/kube/client/client_expansion.go

Codegen support for informers and clients that use the dynamic.Client interface
was dropped in knative/pkg#2742.

* Run update-codegen.sh

* Regenerate pkg/client/injection/kube

* Update 3rd party eventing-latest

* Install cert-manager for e2e tests

Some eventing e2e tests are using eventhub.WithTLS(), which creates
certificates and thus requires cert-manager in their setup. Even this
tests might be skipped, as this is run in their environment setup,
theses test fail if cert-manager is not installed. Therefor we need to
install cert-manager for the e2e tests.

* Update eventing core dependency

* Fix shell lint issue
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. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants