-
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: persist kotlin schemas during deploy #507
Conversation
bfed3b1
to
54ed725
Compare
0bfd460
to
8b8bd98
Compare
cmd/ftl/cmd_deploy.go
Outdated
} | ||
// TODO(worstell): clean up deploy cmd to handle language-specific stuff | ||
if config.Language == "kotlin" { | ||
schemaDirectory := config.Schema |
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.
Is there some reason you thought a directory would be preferable? I thought this would just be a single file containing the module schema as a pb file.
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.
no i'm good with that!
cmd/ftl/cmd_deploy.go
Outdated
Name: config.Module, | ||
} | ||
// TODO(worstell): clean up deploy cmd to handle language-specific stuff | ||
if config.Language == "kotlin" { |
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 will be used for Go as well, so we can remove this condition.
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.
sg - i was thinking we'd eventually merge ftl-go
behavior into ftl deploy
and remove this during the refactor because i think presently this logic would break for Go modules (but maybe ftl deploy
is already broken for go?)
will remove it!
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.
Yeah ftl delpoy doesn't currently work with Go
8b8bd98
to
93c6b4e
Compare
93c6b4e
to
4ed5aca
Compare
No description provided.