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

Serving samples: add instructions to clone code #1203

Merged
merged 35 commits into from May 17, 2019
Merged

Serving samples: add instructions to clone code #1203

merged 35 commits into from May 17, 2019

Conversation

mhamwala
Copy link
Contributor

@mhamwala mhamwala commented Apr 18, 2019 •
edited by RichieEscarez

Fixes #issue-number
#1045
#1269
#1202

Proposed Changes

  • Adding clone instructions to all the serving sample apps in serving/samples

@googlebot googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Apr 18, 2019
@knative-prow-robot knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 18, 2019
@knative-prow-robot
Copy link
Contributor

Hi @mhamwala. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 18, 2019
sebgoa
sebgoa suggested changes Apr 21, 2019
View reviewed changes
docs/serving/samples/hello-world/helloworld-csharp/README.md Outdated
@@ -11,11 +11,15 @@ is not specified, it will use "World" as the TARGET.
and a Docker Hub account configured (we'll use it for a container registry).
- You have installed [.NET Core SDK 2.2](https://www.microsoft.com/net/core).

## Recreating the sample code
## Before you begin
You can either clone the code from this directory using the following commands. Hello world apps are generally more useful if you build them step-by-step therefore you can recreate the source files from this folder by following the steps in the [Recreating the sample code](#Recreating-the-sample-code) section.
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 you need an empty line for good rendering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added space

docs/serving/samples/grpc-ping-go/README.md Show resolved Hide resolved
docs/serving/samples/hello-world/helloworld-scala/README.md Outdated Show resolved Hide resolved
@sebgoa
Copy link
Contributor

sebgoa commented Apr 21, 2019

/ok-to-test

@knative-prow-robot knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 21, 2019
@samodell
Copy link
Contributor

@mhamwala Thanks for the work here! Please implement @sebgoa 's suggested changes, and then we'll get this merged in :)

@mhamwala
Copy link
Contributor Author

Changes addressed PTAL @sebgoa @samodell

@RichieEscarez RichieEscarez self-assigned this May 16, 2019
@RichieEscarez
Copy link
Contributor

Thanks for this @mhamwala! I hope you won't mind but since Im in a bit of a time crunch and want to get this in for the 0.6 cut, im going to commit a few edits to your PR and also address #1269

@mhamwala
Copy link
Contributor Author

@RichieEscarez that's perfectly fine, go for it!

@knative-prow-robot knative-prow-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 17, 2019
@RichieEscarez RichieEscarez mentioned this pull request May 17, 2019
Closed
@RichieEscarez RichieEscarez added approved lgtm Indicates that a PR is ready to be merged. labels May 17, 2019
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: mhamwala

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

@knative-prow-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label May 17, 2019
@RichieEscarez RichieEscarez added the lgtm Indicates that a PR is ready to be merged. label May 17, 2019
@mhamwala
Copy link
Contributor Author

/retest

@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label May 17, 2019
@knative-prow-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@RichieEscarez RichieEscarez added the lgtm Indicates that a PR is ready to be merged. label May 17, 2019
@knative-prow-robot knative-prow-robot merged commit af34e66 into knative:master May 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebgoa sebgoa sebgoa requested changes

@carieshmarie carieshmarie Awaiting requested review from carieshmarie

@samodell samodell Awaiting requested review from samodell

Assignees

@RichieEscarez RichieEscarez

Labels
cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mhamwala @knative-prow-robot @sebgoa @samodell @RichieEscarez @googlebot