Skip to content
This repository was archived by the owner on Apr 13, 2020. It is now read-only.

Conversation

@dennisseah
Copy link
Collaborator

@dennisseah dennisseah commented Apr 4, 2020

  1. converted all errors to cascading error with error id
  2. fix some jest tests
  3. use async-await instead of then, catch

ran integration test successfully

closes microsoft/bedrock#1316

@samiyaakhtar
Copy link
Collaborator

Seeing a regression when i run against this branch:
Screen Shot 2020-04-06 at 11 18 21 AM

@dennisseah
Copy link
Collaborator Author

Seeing a regression when i run against this branch:
Screen Shot 2020-04-06 at 11 18 21 AM

i will test again. does this happens only for -wide?

@samiyaakhtar
Copy link
Collaborator

@dennisseah yes

@dennisseah
Copy link
Collaborator Author

Seeing a regression when i run against this branch:
Screen Shot 2020-04-06 at 11 18 21 AM

i will test again. does this happens only for -wide?

it is also happening for master branch because here https://github.com/CatalystCode/spk/blob/master/src/commands/deployment/get.ts#L244, we have .then and do not have .catch. I will try to fix this. I guess we print warning if we cannot fetch PR, please confirm

@samiyaakhtar
Copy link
Collaborator

@dennisseah interesting, didn't see it happen on master for me but maybe i need to pull again.

@samiyaakhtar
Copy link
Collaborator

@dennisseah i just pulled the latest and it's working on master. Maybe we need to have promise and resolve?
Screen Shot 2020-04-06 at 11 52 02 AM

Copy link
Contributor

@edaena edaena left a comment

Choose a reason for hiding this comment

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

Looks good. Just verify the functionality by running the integration tests before merging.

Copy link
Collaborator

@samiyaakhtar samiyaakhtar left a comment

Choose a reason for hiding this comment

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

Works with -o wide now. Thanks @dennisseah

@dennisseah dennisseah merged commit 57d23a6 into master Apr 6, 2020
@dennisseah dennisseah deleted the 1316 branch April 6, 2020 20:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Have specific error codes and error chain for deployment get command

4 participants