-
Notifications
You must be signed in to change notification settings - Fork 5
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
Turbine-Ruby CLI Stub #539
Conversation
cmd/meroxa/turbine/ruby/run.go
Outdated
|
||
func (t *turbineRbCLI) Run(ctx context.Context) (err error) { | ||
// Run turbine-core gRPC server | ||
return err |
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 return nil, so I recommend adding an error
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.
Sounds good!
cmd/meroxa/turbine/ruby/build.go
Outdated
) | ||
|
||
func (t *turbineRbCLI) Build(ctx context.Context, appName string, platform bool) (string, error) { | ||
return "", nil |
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 think these should return a "not implemented" error
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.
Sounds good!
b701b00
to
8317cfb
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.
@dianadoherty do you think we should a feature flag for the ruby path until we finish it?
because turbine ruby commands are not exposed in this PR, this PR can be merged. But we should definitely add a flag for any future pr |
Description of change
Stubs for turbine-ruby cli. Contains functions needed by interfaces
cli/cmd/meroxa/turbine/interface.go
Line 7 in a3f4517
Fixes #521
Type of change
How was this tested?