Skip to content

Conversation

@abrennan89
Copy link
Contributor

Fixes #2044

Proposed Changes

  • Added nightly executable binary method
  • Added container images
  • Added github / Go method

@abrennan89 abrennan89 added this to the Backlog milestone Dec 12, 2019
@abrennan89 abrennan89 requested a review from samodell December 12, 2019 18:00
@abrennan89 abrennan89 self-assigned this Dec 12, 2019
@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Dec 12, 2019
@knative-prow-robot knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 12, 2019
@RichieEscarez RichieEscarez added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 19, 2019
Copy link
Contributor

@RichieEscarez RichieEscarez left a comment

Choose a reason for hiding this comment

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

@abrennan89 These stand alone installation steps should be moved into the reference section. Which would be used only if someone does not follow the Serving or Eventing install instructions. Meaning that the kn installation should be added to the Serving or Eventing flows and done there by default (not a separate disconnected step).

@samodell
Copy link
Contributor

I think we can at least use a link from Serving/Eventing installs to these instructions, vs. copying them into those topics as well. I'm imagining we'll add an optional step to the installation guides that points to these steps for installing kN.

I've been talking with Richie, and I think what we're thinking is that the /client/ folder should be moved into the /reference/ section, but then this page on installing the client should live in the /install/ folder. I think that's all captured in the nav proposal: https://docs.google.com/spreadsheets/d/1L3EAeTX7RRdZhcQZeNmxwN4DCncuImCXA5A1_KQvwSk/edit#gid=413198505

I apologize for not cross-referencing your first PR with the nav proposal; I should have! Does that seem reasonable, @abrennan89 ?

@abrennan89
Copy link
Contributor Author

Adding a link to these docs from Serving, Eventing and the install section sounds like a good plan @samodell - I was sort of waiting to see if we could include the content instead, which is why I was asking about the re-use mechanism for .md files, but you can let me know if you'd prefer to include the file or just link out to this doc.

If we move the Client section to the Reference section, where would the user stories that we document as tasks live? Would these just be split out and included under Serving and Eventing?

That makes sense to me, but the suggestion to have a separate Client section at the same level as Serving and Eventing also came from @rhuss , so I'd just like to check with you Roland that there are no kn use cases that don't relate directly to Serving or Eventing and would need to be documented in a Client specific section?

Thanks folks for all the input and your patience!

@knative-prow-robot
Copy link
Contributor

knative-prow-robot commented Jan 14, 2020

@abrennan89: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-knative-docs-markdown-link-check 54980e3 link /test pull-knative-docs-markdown-link-check

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

Copy link
Contributor

@samodell samodell left a comment

Choose a reason for hiding this comment

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

Thanks for taking these docs on, Ashleigh!

type: "docs"
---

## Install `kn` using the nightly executable binary
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we also offer installing the latest release of the client? I'm not sure if most users want to download and work with the nightly release.

https://github.com/knative/client/blob/master/docs/README.md has instructions on downloading the most recent release.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also think we should offer some information about who should install the nightly executable, and probably a warning that it may not be super stable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The 'latest' release is also covered under the kn container images section of the docs: https://knative.dev/development/install/install-kn/

I will check in with the CLI team about what to put for a warning/audience note about the nightly executable.

go install ./cmd/kn
```

## `kn` container images
Copy link
Contributor

Choose a reason for hiding this comment

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

What are these container images for? Can we explain what user might do with these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this to cover installing 'latest' - I think it's just how the CLI is installed if you're not using the binaries or Go methods.
The information came from: https://github.com/knative/client/blob/master/docs/README.md
i.e.
To use the kn container image:

Nightly: gcr.io/knative-nightly/knative.dev/client/cmd/kn
Latest release: gcr.io/knative-releases/knative.dev/client/cmd/kn

@samodell samodell removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2020
@samodell
Copy link
Contributor

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 21, 2020
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abrennan89, samodell

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

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

Labels

cla: yes Indicates the PR's author has signed the CLA. kind/cli kind/install lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create Installing kn CLI docs

5 participants