-
Notifications
You must be signed in to change notification settings - Fork 556
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
Refactor NodeJS codegen #4360
Refactor NodeJS codegen #4360
Conversation
1e3c857
to
7f9f4a0
Compare
b31e278
to
d7cd479
Compare
d7cd479
to
80c02da
Compare
Not at all :D |
80c02da
to
b9c258a
Compare
b9c258a
to
efeaa32
Compare
I'll check this one tomorrow! |
efeaa32
to
a00d965
Compare
cc @dolanor ☝️ |
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.
Excellent work, I think you really streamlined it better.
There are a few -}}
that can be avoided I think.
I would need a new version of the client generated code part of the PR to make sure that we don't have regression now that we removed most of codegen unit test.
Can you run a go run ./cmd/client-gen/ --lang nodejs -o ./sdk/nodejs/api/client.gen.ts
and commit it with this PR so we see if we didn't break anything during the refactor?
@dolanor as you're the author of this PR, do you think it could be possible for you to make those changes above in order to ship this PR ? |
431c71a
to
c8f8260
Compare
I'm gonna rebase this and fix conflicts. |
Signed-off-by: Tanguy ⧓ Herrmann <tanguy@dagger.io>
VSCode has an extension for Go template but it only parses files with *.go.tpml or *.gtpl extension. To enable this capabilities, we rename *.ts.tpml to *.ts.gtpl. Simplify types.ts.tpml by merging external template in it. This removes type, type_field_comment and object_comment template. Add comments on some template files. Signed-off-by: Tom Chauveau <tom.chauveau@epitech.eu>
Merge returns template into files. Add comments. Make templates more readable. Signed-off-by: Tom Chauveau <tom.chauveau@epitech.eu>
Signed-off-by: Tom Chauveau <tom.chauveau@epitech.eu>
Signed-off-by: Tom Chauveau <tom.chauveau@epitech.eu>
Signed-off-by: Tanguy ⧓ Herrmann <tanguy@dagger.io>
0021f39
to
56a8c9f
Compare
- (only use it on template comments {{- /* xxx */ -}} or if not possible to use a `{{- "" }}` on the next line. - make templates directive indented by `\t`. Only use ` ` for actual TS code indentation Signed-off-by: Tanguy ⧓ Herrmann <tanguy@dagger.io>
56a8c9f
to
819df4a
Compare
Ok, good to review/merge! |
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.
LGTM
Goal
}}
instead of-}}
as we already have{{-
in general)