Skip to content

Conversation

@jonball4
Copy link
Contributor

Changes

  • Rename the "Response" functions to "KeyedResponse" functions to align with conventions within express-wrapper. These functions use string-based "type" values alongside the payload.
  • Add "Response" functions in place of the original, further aligning with conventions within express-wrapper. These functions use numerical-based "type" values alongside the payload.
  • Add "conflict" / 409 error code to the possible status that can be returned

BREAKING CHANGE: Existing response functions have been renamed to align convention with other packages in this repository.

@jonball4 jonball4 requested a review from a team as a code owner February 11, 2023 04:18
@jonball4 jonball4 force-pushed the update-response-lib branch 2 times, most recently from 31b1c9e to 2662387 Compare February 11, 2023 04:47
@jonball4 jonball4 changed the title feat!: keyed & non-keyed response functions feat(express-wrapper)!: keyed/non-keyed responses Feb 11, 2023
// 500 (internal server error) | INTERNAL | Response.internalError
// 503 (service unavailable) | UNAVAILABLE | Response.serviceUnavailable

export type Status = 200 | 400 | 401 | 403 | 404 | 409 | 429 | 500 | 503;
Copy link

Choose a reason for hiding this comment

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

not a change req but wanted to point out that we specify 405 in the comment above but can't actually throw it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, this was here before fwiw

Copy link

@manav-c manav-c Feb 13, 2023

Choose a reason for hiding this comment

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

yeah I figured

401: 'unauthenticated',
403: 'permissionDenied',
404: 'notFound',
409: 'conflict',
Copy link

Choose a reason for hiding this comment

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

🎸

manav-c
manav-c previously approved these changes Feb 13, 2023
Copy link

@manav-c manav-c left a comment

Choose a reason for hiding this comment

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

LGTM

@jonball4 jonball4 changed the title feat(express-wrapper)!: keyed/non-keyed responses feat(response)!: keyed/non-keyed responses Feb 14, 2023
Changes
-
- Rename the "Response" functions to "KeyedResponse" functions
to align with conventions within express-wrapper. These functions
use string-based "type" values alongside the payload.
- Add "Response" functions in place of the original, further
aligning with conventions within express-wrapper.  These functions
use numerical-based "type" values alongside the payload.
- Add "conflict" / 409 error code to the possible status that can be
returned

BREAKING CHANGE: Existing response functions have been renamed to
align convention with other packages in this repository.
Copy link
Contributor

@bitgopatmcl bitgopatmcl left a comment

Choose a reason for hiding this comment

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

Looks like this breaking change should help with some of the confusing errors that can be generated around responses.

@ericcrosson-bitgo ericcrosson-bitgo merged commit 6a33aba into BitGo:master Feb 14, 2023
@github-actions
Copy link

🎉 This PR is included in version @api-ts/express-wrapper@1.0.14 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version @api-ts/typed-express-router@1.0.11 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version @api-ts/io-ts-http@2.1.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version @api-ts/response@2.0.0 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version @api-ts/openapi-generator@1.0.6 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version @api-ts/superagent-wrapper@1.1.8 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants