Skip to content
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

Update ruby list resource #565

Merged
merged 2 commits into from
Dec 5, 2022
Merged

Update ruby list resource #565

merged 2 commits into from
Dec 5, 2022

Conversation

anna-cross
Copy link
Contributor

@anna-cross anna-cross commented Dec 2, 2022

Description of change

Updating cli side of ruby list resources changes on turbine core. The structure of list resource response has changed, so we need to make appropriate changes to parse them in cli.

Fixes https://github.com/meroxa/turbine-core/issues/36

Type of change

  • Refactor

How was this tested?

  • Unit Tests
  • Tested in staging
  • Tested in minikube

@anna-cross anna-cross force-pushed the ruby_list_resources branch 2 times, most recently from 681baba to bef0eda Compare December 2, 2022 21:27
@anna-cross anna-cross marked this pull request as ready for review December 2, 2022 21:34
Comment on lines 95 to 96
for _, r := range resp.GetResources() {
for _, s := range r.GetDescription() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If api just wants a list of resources, maybe the way core should store them is in just a list format.
for loops inside for loops scare me

@anna-cross anna-cross merged commit 258e4f7 into master Dec 5, 2022
@anna-cross anna-cross deleted the ruby_list_resources branch December 5, 2022 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants