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

feat: Get image from platform #283

Merged
merged 5 commits into from
Mar 31, 2022
Merged

feat: Get image from platform #283

merged 5 commits into from
Mar 31, 2022

Conversation

raulb
Copy link
Member

@raulb raulb commented Mar 21, 2022

Description of change

Fixes https://github.com/meroxa/turbine-project/issues/45

Depends on https://github.com/meroxa/meroxa-go/pull/106

Type of change

  • New feature
  • Bug fix
  • Refactor
  • Documentation

How was this tested?

  • Unit Tests
  • Tested in staging

Demo

See Slack.

@raulb raulb self-assigned this Mar 21, 2022
@raulb raulb force-pushed the raul/build-service-integration branch from c76fd12 to c796e80 Compare March 28, 2022 14:12
}

fmt.Print("Getting status for build: ", build.Uuid)
for {
Copy link
Contributor

Choose a reason for hiding this comment

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

seems dangerous to have no timeout, but i guess it's easy enough to shutdown the CLI

Copy link
Member Author

Choose a reason for hiding this comment

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

@janelletavares that's right, you could still Ctrl+C if you have to. I could still add a time out though

@janelletavares
Copy link
Contributor

i've rebased and updated the dependencies for this branch, as well as addressed lint issues. i feel like Deploy is now too complicated to do much more unit testing in a meaningful way without some potentially convoluted refactoring. i'd prefer to put developer efforts towards acceptance tests as this point. what do you think?

@janelletavares janelletavares marked this pull request as ready for review March 30, 2022 06:10
@raulb
Copy link
Member Author

raulb commented Mar 30, 2022

@janelletavares agreed. Especially since we still need to combine this part to be even more generic so each language does less. Ship it?


fmt.Print("Getting status for build: ", build.Uuid)
for {
fmt.Printf(".")
Copy link
Contributor

Choose a reason for hiding this comment

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

are we using print instead of log because of the newlines here?

Copy link
Member Author

Choose a reason for hiding this comment

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

@janelletavares that's right. I think that should improved though. We'll improve this once we work on https://github.com/meroxa/turbine-project/issues/86.

@raulb raulb force-pushed the raul/build-service-integration branch from e8c3758 to c60ea46 Compare March 31, 2022 05:36
@raulb raulb force-pushed the raul/build-service-integration branch from 25d854b to d2929ab Compare March 31, 2022 05:55
@raulb raulb merged commit 25dd8b8 into master Mar 31, 2022
@raulb raulb deleted the raul/build-service-integration branch March 31, 2022 06:09
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