fixes for ftl init kotlin #517
Conversation
worstell
requested review from
alecthomas,
dhanji and
wesbillman
as code owners
| @@ -10,8 +10,8 @@ data class {{ .Name | camel }}Response(val message: String) | |||
|
|
|||
| class {{ .Name | camel }} { | |||
| @Verb | |||
| fun {{ .Name | lower }}(context: Context, req: {{ .Name | camel }}Request): {{ .Name | camel }}Response { | |||
| @Ingress(Method.GET, "/echo") | |||
| fun echo(context: Context, req: {{ .Name | camel }}Request): {{ .Name | camel }}Response { | |||
There was a problem hiding this comment.
I don't think this should be called echo should it? And we probably don't want methods exposed via @Ingress by default.
There was a problem hiding this comment.
ah i thought this was called echo because the function echoes back the request - maybe we can call it "hello" or something, if the echo naming was a specific reference to the module before?
i'm also good with dynamically naming it after the module, but i thought this could be demonstrative that verb/module names are not necessarily connected
same thing with the ingress, i was thinking it would just surface the annotation syntax to include as much info in the example as possible - but maybe it would confuse people. what do you think about including as a commented out line with some additional comment text like "uncomment to expose this verb to external clients via HTTP"?
There was a problem hiding this comment.
Good point re. echo, makes sense!
Regarding @Ingress, this code shouldn't be a vehicle for documentation/examples because at some point we'll have so many annotations/configuration that we can't possibly include them all in the scaffolded example. Instead, we'll have all of this information in the documentation and tutorials.
There was a problem hiding this comment.
that makes sense - will remove!
worstell
force-pushed
the
worstell/20231020-fix-init-kotlin
branch
from
508a60d to
0f694b5
Compare
No description provided.