Skip to content
This repository has been archived by the owner on Jun 27, 2019. It is now read-only.

Add rawError to callAPI #280

Merged
merged 1 commit into from
Mar 27, 2018
Merged

Add rawError to callAPI #280

merged 1 commit into from
Mar 27, 2018

Conversation

xavdid
Copy link
Contributor

@xavdid xavdid commented Mar 23, 2018

addresses PDE-138

Previously, when the server returned a 400+ error code, we would package the error message up and throw it. This is normally useful, but when we get back a json with errors and want to process them, we were (previously) unable to. It also more correctly identifies when then the issue is an activation one.

This adds a param to callAPI that returns the response object instead of directly throwing an error. This is most relevant in promote, but it will potentially be useful in other places as well. While I was there, I make some of the language more clear too.

Before:

After:


Merging Notes

Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

Pulled and confirmed it works as expected. The code looks good, too!

One minor thing. I see you changed the output message from zapier migrate 1.0.0 1.0.1 [10%] to zapier migrate. Maybe you want to change that in the command help text, too.

@xavdid
Copy link
Contributor Author

xavdid commented Mar 23, 2018

I thought about it, but since that's clearly an example it sort of makes sense to be there. Or if you're more concerned with them matching, i'm happy to.

It would be cool to get some system for keeping the help text and command output in sync. Since the latter happens at runtime it's probably not possible, but it would be useful.

@eliangcs
Copy link
Member

@xavdid since the help text is an example output of the command, I thought it'd better if we can keep them in sync. Just #fyi, not a blocker.

@BrunoBernardino BrunoBernardino removed their request for review March 23, 2018 07:32
Copy link
Contributor

@BrunoBernardino BrunoBernardino left a comment

Choose a reason for hiding this comment

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

Looks simple and a great UX improvement! Good job! I didn't pull the code down, though.

@xavdid xavdid merged commit 1eb7e64 into master Mar 27, 2018
@xavdid xavdid deleted the promotion-cleanup branch March 27, 2018 03:34
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.

3 participants