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

Initial PR for kn trigger commands #19756

Merged
merged 1 commit into from
Mar 19, 2020
Merged

Initial PR for kn trigger commands #19756

merged 1 commit into from
Mar 19, 2020

Conversation

abrennan89
Copy link
Contributor

No description provided.

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 14, 2020
@openshift-docs-preview-bot
Copy link

The preview will be available shortly at:

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 21, 2020
@abrennan89 abrennan89 force-pushed the trigger branch 2 times, most recently from 7eb7183 to 4078f56 Compare February 28, 2020 15:40
@abrennan89 abrennan89 force-pushed the trigger branch 2 times, most recently from 1cbe7fc to 208c353 Compare February 28, 2020 16:15
@navidshaikh
Copy link
Contributor

/cc @rhuss

@abrennan89 abrennan89 force-pushed the trigger branch 2 times, most recently from 45097a1 to e7453d1 Compare March 3, 2020 18:54
mvinkler
mvinkler reviewed Mar 18, 2020
View reviewed changes
serverless/knative_eventing/serverless-kn-trigger.adoc

To create a trigger, enter the following command:
----
$ kn trigger create <TRIGGER-NAME> --broker <BROKER-NAME> --filter <KEY=VALUE> --sink <SINK>
Copy link

Choose a reason for hiding this comment

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

--filter shouldn't be mandatory, @dsimansk filed a bug for it yesterday, so it should be updated here as well

Copy link

Choose a reason for hiding this comment

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

As Michal stated above. We should probably note that current kn requires filter. So no change for now, until it's fixed.

dsimansk
dsimansk reviewed Mar 18, 2020
View reviewed changes
modules/kn-trigger-update.adoc
You can also remove a filter attribute from a trigger.
For example, you can remove the filter attribute with key `type`:
----
$ kn trigger update mytrigger --filter type-
Copy link

Choose a reason for hiding this comment

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

The removal of filter with --filter type- doesn't work currently.

@abrennan89 abrennan89 force-pushed the trigger branch 2 times, most recently from 57e9d23 to ab78e77 Compare March 18, 2020 18:07
@abrennan89
Copy link
Contributor Author

The --filter issues are scheduled to be resolved before GA, so this PR should be fine for now since it won't actually be published/show up in docs until then anyway.
We can make any tweaks later ahead of the GA release - for now, we should go ahead and merge this if possible to prevent blocking other work towards GA that depends on trigger docs being merged.
cc @dsimansk @rhuss @mvinkler

@mvinkler
Copy link

QE approved

@abrennan89 abrennan89 changed the title [WIP] Initial PR for kn trigger commands Initial PR for kn trigger commands Mar 19, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 19, 2020
@ahardin-rh ahardin-rh added the peer-review-done Signifies that the peer review team has reviewed this PR label Mar 19, 2020
@ahardin-rh
Copy link
Contributor

@abrennan89 Just a few minor comments from me. Overall, looks good!

@abrennan89
Copy link
Contributor Author

Thanks @ahardin-rh - can we get this merged today please? It applies for 4.3 and 4.4

@ahardin-rh ahardin-rh added this to the Next Release milestone Mar 19, 2020
@ahardin-rh ahardin-rh merged commit df0b4f9 into openshift:master Mar 19, 2020
@ahardin-rh
Copy link
Contributor

/cherrypick enterprise-4.3

@ahardin-rh
Copy link
Contributor

/cherrypick enterprise-4.4

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Mar 19, 2020
Merged
@openshift-cherrypick-robot
Copy link

@ahardin-rh: new pull request created: #20595

In response to this:

/cherrypick enterprise-4.3

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/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Mar 19, 2020
Merged
@openshift-cherrypick-robot
Copy link

@ahardin-rh: new pull request created: #20596

In response to this:

/cherrypick enterprise-4.4

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/test-infra repository.

@abrennan89 abrennan89 deleted the trigger branch June 10, 2020 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahardin-rh ahardin-rh ahardin-rh left review comments

@mvinkler mvinkler mvinkler left review comments

@dsimansk dsimansk dsimansk left review comments

@navidshaikh navidshaikh navidshaikh left review comments

@rhuss rhuss Awaiting requested review from rhuss

Assignees
No one assigned
Labels
branch/enterprise-4.3 branch/enterprise-4.4 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
Next Release
Development

Successfully merging this pull request may close these issues.
8 participants
@abrennan89 @openshift-docs-preview-bot @navidshaikh @mvinkler @ahardin-rh @openshift-cherrypick-robot @dsimansk @openshift-ci-robot