-
Notifications
You must be signed in to change notification settings - Fork 122
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
thread api error bodies through HttpError, and use for starter tier quota message #725
Conversation
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.
Much nicer developer experience than our current error messaging, and much nicer implementation than mine in #698!! But I’m happy I tried it before you so I can review with more context. 😅
src/deploy.ts
Outdated
wrapAnsi(`Could not create project: ${error instanceof Error ? error.message : error}`, effects.outputColumns) | ||
); | ||
} | ||
effects.clack.outro(yellow("Deploy cancelled")); |
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.
We have 13 instances of “cancelled” in the create and deploy scripts, but in American English it’s more standard to write “canceled”.
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.
Ambiguous double letters are the bane of my spelling life.
wrapAnsi( | ||
`The Starter tier can only deploy one project. Upgrade to unlimited projects at ${link( | ||
`https://observablehq.com/team/@${deployTarget.workspace.login}/settings` | ||
)}`, | ||
effects.outputColumns - 4 | ||
) |
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.
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.
Yes, the idea is to eventually do this everywhere, and both Mike and Visnu recommended wrapAnsi
. This is sort of a toe in the water to make sure this approach works in practice. In the future I hope to do it automatically in a Clack wrapper.
Thanks for writing the copy for this, @toph!