Added a serving sample for dart #452
Conversation
knative-prow-robot
added
the
size/L
|
Hi @jonasfj. 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 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
added
the
needs-ok-to-test
|
|
||
| A simple web app written in Dart that you can use for testing. | ||
| It reads in the env variable `TARGET` and prints "Hello World: ${TARGET". If | ||
| `TARGET` is not specified, it will use `"NOT SPECIFIED"` as `TARGET`. |
There was a problem hiding this comment.
I just updated the other samples so that they display Hello World if no TARGET is specified, see #443. Can you also update yours?
There was a problem hiding this comment.
Oh, sorry I missed the feedback... should be done now :)
jonasfj
force-pushed
the
dart-serving-sample
branch
from
7f869e2 to
9d79c1a
Compare
| FROM google/dart-runtime | ||
|
|
||
| # google/dart-runtime expects us to listen on port 8080 | ||
| ENV PORT 8080 No newline at end of file |
There was a problem hiding this comment.
This is not needed, Knative injects a PORT env var with the appropriate value.
| var port = int.tryParse(Platform.environment['PORT']); | ||
|
|
||
| // Read $TARGET from environment variable | ||
| var target = Platform.environment['TARGET'] ?? 'NOT SPECIFIED'; |
There was a problem hiding this comment.
Can you update "World" here to?
| FROM google/dart-runtime | ||
|
|
||
| # google/dart-runtime expects us to listen on port 8080 | ||
| ENV PORT 8080 |
| FROM google/dart-runtime | ||
|
|
||
| # google/dart-runtime expects us to listen on port 8080 | ||
| ENV PORT 8080 No newline at end of file |
There was a problem hiding this comment.
Missing newline at the end?
| image: docker.io/{username}/helloworld-dart | ||
| env: | ||
| - name: TARGET | ||
| value: "Dart Sample v1" No newline at end of file |
There was a problem hiding this comment.
Missing newline at the end?
| import 'package:shelf/shelf_io.dart'; | ||
|
|
||
| void main() { | ||
| // Find port to listen on (defined in Dockerfile) |
There was a problem hiding this comment.
Terminate comments (throughout) with a full stop .
| return Response.ok('Hello $target'); | ||
| }); | ||
|
|
||
| // Server on port |
There was a problem hiding this comment.
'Server on port' reads like a partial sentence.
| @@ -0,0 +1,165 @@ | |||
| # Hello World - Dart sample | |||
|
|
|||
| A simple web app written in Dart that you can use for testing. | |||
There was a problem hiding this comment.
Please s/Dart/the [Dart](www.dartlang.org) programming language/
| ## Recreating the sample code | ||
|
|
||
| While you can clone all of the code from this directory, it is useful to know how | ||
| to build a hello world dart application step-by-step. This application can be |
|
|
||
| ## Building and deploying the sample | ||
|
|
||
| Once you have recreated the sample code files (or used the files in the sample |
There was a problem hiding this comment.
The word sample here is not an identifier, why would you format as such?
There was a problem hiding this comment.
I thought sample was a named folder; if not, just leave as is
jonasfj
force-pushed
the
dart-serving-sample
branch
from
9d79c1a to
cca42d4
Compare
|
@mchmarny @vaikas-google can we merge this? |
|
paging @mchmarny @vaikas-google |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jonasfj, mit-mit, vaikas-google 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 |
|
/lgtm |
5 participants
Adds a
helloworld-dartsample based onhelloworld-nodejs