Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Remove v1alpha1 support for Channel #1853

Merged
merged 1 commit into from
Oct 26, 2020
Merged

Remove v1alpha1 support for Channel #1853

merged 1 commit into from
Oct 26, 2020

Conversation

eclipselu
Copy link
Contributor

@eclipselu eclipselu commented Oct 22, 2020 •
edited
Loading

Part of #1544

Proposed Changes

  • 🗑️ Remove v1alpha1 Channel

Release Note

Dropped v1alpha1 support for the following resources:

- Channel

Docs

@google-cla google-cla bot added the cla: yes (override cla status due to multiple authors bug) label Oct 22, 2020
@eclipselu eclipselu marked this pull request as draft October 22, 2020 16:24
@eclipselu eclipselu marked this pull request as ready for review October 22, 2020 17:29
@eclipselu eclipselu changed the title Remove v1alpha1 Channel [WIP] Remove v1alpha1 Channel Oct 22, 2020
@eclipselu eclipselu changed the title [WIP] Remove v1alpha1 Channel Remove v1alpha1 Channel Oct 22, 2020
@eclipselu
Copy link
Contributor Author

Running ./hack/update-codegen.sh after removing v1alpha1 api for channels does not remove the generated client code, I manually removed them.

capri-xiyue
capri-xiyue reviewed Oct 22, 2020
View reviewed changes
test/e2e/main_test.go Show resolved Hide resolved
cmd/webhook/main.go Show resolved Hide resolved
cmd/webhook/main.go
@@ -274,15 +269,6 @@ func newConversionController(ctx context.Context, cmw configmap.Watcher, brokers
inteventsv1_: &inteventsv1.Topic{},
},
},
// messaging
messagingv1alpha1.Kind("Channel"): {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of removing it, maybe we keep v1beat1 channel here? I know it is meaningless since now channel only has one version v1bea1 and there is no need to do conversion webhook here. Any suggestions @Harwayne ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ping @Harwayne

Copy link
Contributor

Choose a reason for hiding this comment

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

I think removing it is fine. It will be easy enough to add back when we want to, as there are many other examples to draw upon.

@eclipselu
Copy link
Contributor Author

/test pull-google-knative-gcp-conformance-tests

@eclipselu
Copy link
Contributor Author

/retest

@eclipselu eclipselu mentioned this pull request Oct 23, 2020
Merged
@capri-xiyue
Copy link
Contributor

/lgtm

@capri-xiyue
Copy link
Contributor

/unhold

@eclipselu eclipselu changed the title Remove v1alpha1 Channel Remove v1alpha1 support for Channel Oct 23, 2020
@eclipselu eclipselu assigned Harwayne and unassigned liu-cong Oct 26, 2020
Harwayne
Harwayne reviewed Oct 26, 2020
View reviewed changes
Copy link
Contributor

@Harwayne Harwayne left a comment

Choose a reason for hiding this comment

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

/approve

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eclipselu, Harwayne

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

@eclipselu eclipselu removed the request for review from liu-cong October 26, 2020 17:58
@knative-prow-robot knative-prow-robot merged commit b4692ad into google:master Oct 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Harwayne Harwayne Harwayne left review comments

@capri-xiyue capri-xiyue capri-xiyue left review comments

Assignees

@Harwayne Harwayne

@capri-xiyue capri-xiyue

Labels
approved cla: yes (override cla status due to multiple authors bug) lgtm size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eclipselu @capri-xiyue @knative-prow-robot @Harwayne @liu-cong