-
Notifications
You must be signed in to change notification settings - Fork 8
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.
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
feat: generate kotlin external modules from FTL tooling #988
feat: generate kotlin external modules from FTL tooling #988
Conversation
e8d0020
to
06dedc8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one Lizzy!
}, scaffolder.Functions(funcs)) | ||
} | ||
|
||
var scaffoldFuncs = scaffolder.FuncMap{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions are already defined in a couple of places. Let's factor them into a single location and include them by default in internal.ScaffoldZip
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scaffolder.Functions()
is cumulative, so the bespoke functions can still be added in each individual location.
val parameterizedDataRef: ParamTestData<T>, | ||
val withAlias: String, | ||
val unit: Unit, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice.
{{- if $imports}} | ||
{{range $import := $imports}} | ||
import {{$import}} | ||
{{- end}}{{end}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite difficult to read. Let's introduce some whitespace here between the different semantic sections. The intent of using {{-
and - }}
, is to allow the template to have meaningful whitespace while still generating clean output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated! hopefully clearer chunks now
.removeSuffix("*/") | ||
.lines() | ||
.map { it.trim().removePrefix("*").trim() } | ||
.filter { it.isNotEmpty() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to remove empty lines in the comments do we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah the thinking here was to filter out the lines that become empty after removing the javadoc start/end markers /**
and */
- but good point, we don't want to lose intentional line spacing. I've updated to only filter out the lines that contain start/end markers with nothing else
8cfe6b8
to
c575fab
Compare
c7501aa
to
b8280e0
Compare
fixes #970
This implementation diverges from the kotlin-runtime module generator logic in that it omits
IngressHttp
orJson
annotations in external modules. I don't think we need those for the generated external modules.Will follow with a change to remove the module generator entirely and stop releasing its JAR.