Switch helloworld-go with kn #2807

mattmoor · 2020-09-11T22:31:00Z

WIP as this is based on #2805

mattmoor · 2020-09-11T22:39:39Z

@abrennan89 @rhuss I did a similar thing in this PR with helloworld-go, check out this section:
https://docs-2807.default.docs-on-the-rocks.io/docs/serving/samples/hello-world/helloworld-go/#building-and-deploying-the-sample

mattmoor · 2020-09-16T22:32:40Z

Ok, this sample is RFAL here: https://docs-2807.default.docs-on-the-rocks.io/docs/serving/samples/hello-world/helloworld-go/#building-and-deploying-the-sample

I updated the tab default to kn here as well

mattmoor · 2020-09-17T14:20:59Z

I created https://docs.google.com/spreadsheets/d/1Nbp1nb0V1l-8ZfdGnXZMfrcgt6POS9ZbT94Sa8GGFhc/edit#gid=0 to crowd-source going wide on these samples once we have a canonical one landed.

abrennan89

@mattmoor left a bunch of comments.
In terms of doc structure, I'm really not sure everything in one big doc like this with tabs for samples is the way to go.
I think it's less readable than just creating one focused procedure for each, e.g.

"Using the helloworld-go sample app with kn CLI"
"Using the helloworld-go sample app with YAML"

that live in the same page under the heading of helloworld-go sample.
Even the prerequisites doc'd this way aren't really correct because they don't mention installing kn and that is a pre-req but only if you use the procedure using kn, so I think they should be separate.

abrennan89 · 2020-09-17T15:33:51Z

docs/serving/samples/hello-world/helloworld-go/index.md

+A simple web app written in Go that you can use for testing. It reads in an env
+variable `TARGET` and prints `Hello ${TARGET}!`. If `TARGET` is not specified,
+it will use `World` as the `TARGET`.
+
+Follow the steps below to create the sample code and then deploy the app to your
+cluster. You can also download a working copy of the sample, by running the
+following commands:
+
+```shell
+git clone -b "{{< branch >}}" https://github.com/knative/docs knative-docs
+cd knative-docs/docs/serving/samples/hello-world/helloworld-go
+```


Suggested change

A simple web app written in Go that you can use for testing. It reads in an env

variable `TARGET` and prints `Hello ${TARGET}!`. If `TARGET` is not specified,

it will use `World` as the `TARGET`.

Follow the steps below to create the sample code and then deploy the app to your

cluster. You can also download a working copy of the sample, by running the

following commands:

```shell

git clone -b "{{< branch >}}" https://github.com/knative/docs knative-docs

cd knative-docs/docs/serving/samples/hello-world/helloworld-go

```

This guide describes the steps required to to create the `helloworld-go` sample app and deploy it to your

cluster.

The sample app reads a `TARGET` environment variable, and prints `Hello ${TARGET}!`.

If `TARGET` is not specified, `World` is used as the default value.

You can also download a working copy of the sample, by running the

following commands:

```shell

git clone -b "{{< branch >}}" https://github.com/knative/docs knative-docs

cd knative-docs/docs/serving/samples/hello-world/helloworld-go

I would move this part about the working copy to the procedure part, maybe split 1. into two options, i.e. you can copy/paste the code or you can use the "working copy" - also I'd make it clearer what the distinction is here, it's not really clear how stable or unstable each option is

I think I got it all, but LMK if I missed anything.

docs/serving/samples/hello-world/helloworld-go/index.md

abrennan89 · 2020-09-17T15:45:24Z

docs/serving/samples/hello-world/helloworld-go/index.md

+cd knative-docs/docs/serving/samples/hello-world/helloworld-go
+```
+
+## Before you begin


How do folks feel about standardizing headings for these samples as follows:

Prerequisites

Procedure

Verification steps

Deleting the sample code

?

+1 for the same document structure across all examples. The suggestion looks good to me.

I followed the headings from @rhuss PR, which I now realize are difference from here. +1 for consistency, I just need to know what we want the shape to be 😅

docs/serving/samples/hello-world/helloworld-go/index.md

abrennan89 · 2020-09-17T16:02:19Z

docs/serving/samples/hello-world/helloworld-go/index.md

+   Run the following command to find the domain URL for your service:
+
+   ```shell
+   kubectl get ksvc helloworld-go  --output=custom-columns=NAME:.metadata.name,URL:.status.url
+   ```
+
+   Example:
+
+   ```shell
+    NAME                URL
+    helloworld-go       http://helloworld-go.default.1.2.3.4.xip.io
+   ```


No. 4; also I feel like this should be a verification step instead.
I'm not sure that tabs are the best way to display this info at all tbh...

What do you suggest? We'd been talking about tabs for a while, which is why I'd bit the bullet here. Definitely open to suggestions.

docs/serving/samples/hello-world/helloworld-go/index.md

rhuss

Looks good, but I would like to add the following suggestions:

Kn should be added to the prerequisites.
The command to show the URL can be also performed with kn describe service helloworld-go -o url
The service can be deleted with kn service delete helloworld-go

The latter two commands can be also added as tabs. I'm just preparing an example for the shell sample, which could be used here as blueprint, too.

rhuss · 2020-09-25T10:34:25Z

I really wonder why the test is passing as README.md is removed but sampleconsistency test explicitly tries to read it, so that the e2e test should fail here.

/test pull-knative-docs-unit-tests

rhuss · 2020-09-25T10:41:06Z

ah, here we go ;-) test fails now, too. I'm going to fix this issue / check along with #2832

google-cla · 2020-11-06T04:37:54Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2020-11-06T05:07:57Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

mattmoor

ok, I think I got all the feedback incorporated. A lot of this was carryover from README.md -> index.md, so we may want to do a pass over the other languages to ensure we're consistently applying the feedback to all of them.

I just pushed a new commit with the changes, and I've rebased this on #2832 where @rhuss fixed the test. I'll check the staged site shortly and make sure things look alright.

docs/serving/samples/hello-world/helloworld-go/index.md

mattmoor · 2020-11-06T04:49:40Z

docs/serving/samples/hello-world/helloworld-go/index.md

+A simple web app written in Go that you can use for testing. It reads in an env
+variable `TARGET` and prints `Hello ${TARGET}!`. If `TARGET` is not specified,
+it will use `World` as the `TARGET`.
+
+Follow the steps below to create the sample code and then deploy the app to your
+cluster. You can also download a working copy of the sample, by running the
+following commands:
+
+```shell
+git clone -b "{{< branch >}}" https://github.com/knative/docs knative-docs
+cd knative-docs/docs/serving/samples/hello-world/helloworld-go
+```


I think I got it all, but LMK if I missed anything.

docs/serving/samples/hello-world/helloworld-go/index.md

mattmoor · 2020-11-06T05:05:39Z

docs/serving/samples/hello-world/helloworld-go/index.md

+   Run the following command to find the domain URL for your service:
+
+   ```shell
+   kubectl get ksvc helloworld-go  --output=custom-columns=NAME:.metadata.name,URL:.status.url
+   ```
+
+   Example:
+
+   ```shell
+    NAME                URL
+    helloworld-go       http://helloworld-go.default.1.2.3.4.xip.io
+   ```


What do you suggest? We'd been talking about tabs for a while, which is why I'd bit the bullet here. Definitely open to suggestions.

mattmoor · 2020-11-06T05:06:28Z

docs/serving/samples/hello-world/helloworld-go/index.md

+   # Build the container on your local machine
+   docker build -t {username}/helloworld-go .
+
+   # Push the container to docker registry
+   docker push {username}/helloworld-go


Ack, we probably need to update the other samples.

google-cla · 2020-11-06T05:10:26Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2020-11-06T05:24:49Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

mattmoor · 2020-11-06T05:28:44Z

Needs #3004 to fix the local build, but when I pulled it up it looked fine (after some tweaks).

docs/serving/samples/hello-world/helloworld-go/index.md

abrennan89

Minor suggestions, otherwise lgtm :)

abrennan89

/hold
/lgtm

Thanks Matt! I'm fine with merging this now 🙂 sticking a hold on it to make sure you're done with commits first, but feel free to remove

knative-prow-robot · 2020-11-06T16:48:06Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

~~OWNERS~~ [abrennan89,mattmoor]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mattmoor · 2020-11-06T16:54:07Z

/unhold

Yep, done for now. Thanks, @abrennan89 and sorry this took so long for me to circle back 😅

abrennan89 · 2020-11-06T17:00:42Z

Yep, done for now. Thanks, @abrennan89 and sorry this took so long for me to circle back sweat_smile

lol no problem at all, thank you!

@abrennan89

* Try helloworld-go with kn * Add kn as a prereq. * Incorporate Ashleigh's feedback * Fix @abrennan89 feedback

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Sep 11, 2020

knative-prow-robot requested review from markusthoemmes and tcnghia September 11, 2020 22:31

knative-prow-robot added the approved label Sep 11, 2020

mattmoor changed the title ~~Helloworld go kn~~ Try helloworld-go with kn Sep 11, 2020

knative-prow-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 11, 2020

mattmoor changed the title ~~Try helloworld-go with kn~~ [WIP] Try helloworld-go with kn Sep 11, 2020

knative-prow-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 11, 2020

mattmoor force-pushed the helloworld-go-kn branch from f9a2790 to c36b2aa Compare September 11, 2020 22:35

mattmoor force-pushed the helloworld-go-kn branch from c36b2aa to 461cb5b Compare September 16, 2020 22:21

mattmoor changed the title ~~[WIP] Try helloworld-go with kn~~ Try helloworld-go with kn Sep 16, 2020

mattmoor force-pushed the helloworld-go-kn branch from 461cb5b to bf1668c Compare September 16, 2020 22:30

mattmoor assigned rhuss and abrennan89 Sep 16, 2020

mattmoor changed the title ~~Try helloworld-go with kn~~ Switch helloworld-go with kn Sep 17, 2020

abrennan89 reviewed Sep 17, 2020

View reviewed changes

rhuss reviewed Sep 18, 2020

View reviewed changes

abrennan89 requested a review from RichieEscarez September 28, 2020 14:18

abrennan89 removed their assignment Sep 28, 2020

mattmoor force-pushed the helloworld-go-kn branch from bf1668c to 6b73224 Compare November 6, 2020 04:37

google-cla bot removed the cla: yes Indicates the PR's author has signed the CLA. label Nov 6, 2020

knative-prow-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 6, 2020

mattmoor commented Nov 6, 2020

View reviewed changes

mattmoor force-pushed the helloworld-go-kn branch from 2c45a74 to f205b7b Compare November 6, 2020 05:10

mattmoor force-pushed the helloworld-go-kn branch from f205b7b to 197024a Compare November 6, 2020 05:24

mattmoor added 3 commits November 6, 2020 08:06

Try helloworld-go with kn

d800251

Add kn as a prereq.

8e4587b

Incorporate Ashleigh's feedback

0f22cfd

mattmoor force-pushed the helloworld-go-kn branch from 197024a to 0f22cfd Compare November 6, 2020 16:07

google-cla bot added cla: yes Indicates the PR's author has signed the CLA. and removed cla: no Indicates the PR's author has not signed the CLA. labels Nov 6, 2020

knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 6, 2020

abrennan89 reviewed Nov 6, 2020

View reviewed changes

docs/serving/samples/hello-world/helloworld-go/index.md Outdated Show resolved Hide resolved

abrennan89 reviewed Nov 6, 2020

View reviewed changes

docs/serving/samples/hello-world/helloworld-go/index.md Outdated Show resolved Hide resolved

abrennan89 reviewed Nov 6, 2020

View reviewed changes

Fix @abrennan89 feedback

e3060bc

abrennan89 approved these changes Nov 6, 2020

View reviewed changes

knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 6, 2020

knative-prow-robot assigned abrennan89 Nov 6, 2020

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 6, 2020

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 6, 2020

knative-prow-robot merged commit 9f2138f into knative:master Nov 6, 2020

RichieEscarez pushed a commit to RichieEscarez/docs that referenced this pull request Mar 6, 2021

Switch helloworld-go with kn (knative#2807)

ed6fa92

* Try helloworld-go with kn * Add kn as a prereq. * Incorporate Ashleigh's feedback * Fix @abrennan89 feedback

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch helloworld-go with kn #2807

Switch helloworld-go with kn #2807

mattmoor commented Sep 11, 2020 •

edited

Loading

mattmoor commented Sep 11, 2020

mattmoor commented Sep 16, 2020

mattmoor commented Sep 17, 2020

abrennan89 left a comment

abrennan89 Sep 17, 2020

abrennan89 Sep 17, 2020

mattmoor Nov 6, 2020

abrennan89 Sep 17, 2020

rhuss Sep 18, 2020

mattmoor Nov 6, 2020

abrennan89 Sep 17, 2020

mattmoor Nov 6, 2020

rhuss left a comment

rhuss commented Sep 25, 2020

rhuss commented Sep 25, 2020

google-cla bot commented Nov 6, 2020

google-cla bot commented Nov 6, 2020

mattmoor left a comment

mattmoor Nov 6, 2020

mattmoor Nov 6, 2020

mattmoor Nov 6, 2020

google-cla bot commented Nov 6, 2020

google-cla bot commented Nov 6, 2020

mattmoor commented Nov 6, 2020

abrennan89 left a comment

abrennan89 left a comment

knative-prow-robot commented Nov 6, 2020

mattmoor commented Nov 6, 2020

abrennan89 commented Nov 6, 2020

Switch helloworld-go with kn #2807

Switch helloworld-go with kn #2807

Conversation

mattmoor commented Sep 11, 2020 • edited Loading

mattmoor commented Sep 11, 2020

mattmoor commented Sep 16, 2020

mattmoor commented Sep 17, 2020

abrennan89 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prerequisites

Procedure

Verification steps

Deleting the sample code

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhuss left a comment

Choose a reason for hiding this comment

rhuss commented Sep 25, 2020

rhuss commented Sep 25, 2020

google-cla bot commented Nov 6, 2020

google-cla bot commented Nov 6, 2020

mattmoor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

google-cla bot commented Nov 6, 2020

google-cla bot commented Nov 6, 2020

mattmoor commented Nov 6, 2020

abrennan89 left a comment

Choose a reason for hiding this comment

abrennan89 left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Nov 6, 2020

mattmoor commented Nov 6, 2020

abrennan89 commented Nov 6, 2020

mattmoor commented Sep 11, 2020 •

edited

Loading