Skip to content

Conversation

@steren
Copy link
Contributor

@steren steren commented Oct 7, 2018

When no env var is provided (for example because the developer only has deployed the code without using the service.yaml), instead of displaying "Hello World: NOT SPECIFIED", display the more friendly message "Hello Word".
This means that when the TARGET env var is set, instead of displaying "Hello World: ", the samples will display "Hello ", which seems totally OK.

All sample changes have been tested.

@steren steren requested a review from rgregg October 7, 2018 01:55
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 7, 2018
@steren
Copy link
Contributor Author

steren commented Oct 7, 2018

Looking at the Test failure, I conclude this is not due to my changes.

@vdemeester
Copy link
Contributor

/test pull-knative-docs-integration-tests

@mattmoor
Copy link
Member

mattmoor commented Oct 8, 2018

/test pull-knative-docs-integration-tests
/approve
/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 8, 2018
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@bbrowning
Copy link
Contributor

The integration test failure was unrelated to this change and due to an issue impacting all PRs in the docs repo that is now fixed.

/test pull-knative-docs-integration-tests

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

Labels

lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants