Skip to content

Add kn channel docs #26282

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
Merged
bobfuru merged 1 commit into from
Oct 27, 2020
Merged

Add kn channel docs #26282

bobfuru merged 1 commit into from
Oct 27, 2020

Conversation

@abrennan89
Copy link
Contributor

@abrennan89 abrennan89 commented Oct 9, 2020 •
edited
Loading

@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 Oct 9, 2020
@openshift-docs-preview-bot
Copy link

openshift-docs-preview-bot commented Oct 9, 2020

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 Oct 13, 2020
@abrennan89 abrennan89 force-pushed the knchannels branch 5 times, most recently from a702fdf to cce7c6a Compare October 14, 2020 13:59
@abrennan89 abrennan89 changed the title [WIP] Add kn channel docs Add kn channel docs Oct 14, 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 Oct 14, 2020
rhuss
rhuss reviewed Oct 14, 2020
View reviewed changes
Copy link
Contributor

@rhuss rhuss left a comment

Choose a reason for hiding this comment

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

Looks good mostly, I have some comments about some things that could maybe need some more clarification.

As mentioned I'm not sure whether there is already documentation about the kn configuration file and how you eg. can define sink prefixes within it. If so, we should also add how to define channel prefixes.

modules/serverless-create-channel-kn.adoc Outdated
Copy link
Contributor

@rhuss rhuss Oct 14, 2020

Choose a reason for hiding this comment

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

I think we have also the possibility to define prefixes on our own in the configuration file (is this correct @navidshaikh ?), plus there is one predefined prefix imc: for an InMemoryChannel.

@abrennan89 do we already have a section where we describe the configuration file for kn ? If yes, I would add this, if not, we need a JIRA for adding this to the docs, too. I can deliver draft input for this.

Copy link
Contributor

@rhuss rhuss Oct 14, 2020

Choose a reason for hiding this comment

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

Maybe we should also explain what happens when now type is given: In this case a default type is selected which is configured clusterwide.

modules/serverless-creating-subscriptions-kn.adoc Outdated
Copy link
Contributor

@rhuss rhuss Oct 14, 2020

Choose a reason for hiding this comment

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

I wonder where we should add that <GVK>: is optional (could be also a prefix like imc:), and if not given (as in the example below), then the default channel is used ?

modules/serverless-creating-subscriptions-kn.adoc Outdated
Copy link
Contributor

@rhuss rhuss Oct 14, 2020

Choose a reason for hiding this comment

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

I think a reference which describes the sink prefix (which can also be optional) would be very helpful here.

@abrennan89
Copy link
Contributor Author

abrennan89 commented Oct 15, 2020

We will follow up Roland's comments and more in depth prefix docs in the CLI reference guide work.
Jira is open for this now: https://issues.redhat.com/browse/SRVCOM-1075

cardil
cardil approved these changes Oct 22, 2020
View reviewed changes
Copy link
Contributor

@cardil cardil left a comment

Choose a reason for hiding this comment

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

I've got nothing apart from @rhuss already pointed.

/lgtm

from QE

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 22, 2020
@bobfuru bobfuru modified the milestones: Future Release, Next Release Oct 26, 2020
bobfuru
bobfuru reviewed Oct 26, 2020
View reviewed changes
Copy link
Contributor

@bobfuru bobfuru left a comment

Choose a reason for hiding this comment

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

A few comments and otherwise LGTM!

@bobfuru bobfuru added the peer-review-done Signifies that the peer review team has reviewed this PR label Oct 26, 2020
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 26, 2020
@openshift-ci-robot
Copy link

openshift-ci-robot commented Oct 26, 2020

New changes are detected. LGTM label has been removed.

abrennan89
abrennan89 commented Oct 26, 2020
View reviewed changes
Copy link
Contributor Author

@abrennan89 abrennan89 left a comment

Choose a reason for hiding this comment

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

Thanks @bobfuru - should be up to date now.
I've commented out the sections that are for 1.11.0 (due in Nov) so this is OK to merge once the docs freeze is over.
This is for 4.6 only.

@bobfuru bobfuru added serverless Label for all Serverless PRs branch/enterprise-4.7 and removed branch/enterprise-4.5 labels Oct 26, 2020
@bobfuru bobfuru merged commit fb9518b into openshift:master Oct 27, 2020
@bobfuru
Copy link
Contributor

bobfuru commented Oct 27, 2020 •
edited by openshift-ci-robot
Loading

/cherrypick enterprise-4.6

@bobfuru
Copy link
Contributor

bobfuru commented Oct 27, 2020 •
edited by openshift-ci-robot
Loading

/cherrypick enterprise-4.7

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Oct 27, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Oct 27, 2020 •
edited by openshift-ci-robot
Loading

@bobfuru: new pull request created: #26786

Details

In response to this:

/cherrypick enterprise-4.6

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 Oct 27, 2020
Merged
@openshift-cherrypick-robot
Copy link

openshift-cherrypick-robot commented Oct 27, 2020 •
edited by openshift-ci-robot
Loading

@bobfuru: new pull request created: #26787

Details

In response to this:

/cherrypick enterprise-4.7

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 knchannels branch December 17, 2021 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@rhuss rhuss rhuss left review comments

@bobfuru bobfuru bobfuru left review comments

@cardil cardil cardil approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@cardil cardil

Labels

branch/enterprise-4.6 branch/enterprise-4.7 peer-review-done Signifies that the peer review team has reviewed this PR serverless Label for all Serverless PRs 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.

7 participants

@abrennan89 @openshift-docs-preview-bot @openshift-ci-robot @bobfuru @openshift-cherrypick-robot @rhuss @cardil