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

JSONCoder extensions... Potentially dangerous. #226

Open
fabianfett opened this issue Aug 26, 2021 · 1 comment
Open

JSONCoder extensions... Potentially dangerous. #226

fabianfett opened this issue Aug 26, 2021 · 1 comment
Labels
⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0
Milestone

Comments

@fabianfett
Copy link
Member

We do something potentially dangerous here:
#144

We extend a type we don't own with an unprefixed method that only takes arguments we don't own.

@fabianfett fabianfett changed the title Public API discussion JSONCoder extensions... Potentially dangerous. Aug 26, 2021
@stevapple
Copy link
Contributor

stevapple commented Aug 26, 2021

I agree that this extension should go away... APIGateway issue could have a better solution.

I'm not saying that I've already got a better solution. I'm just showing how I handle this in swift-tencent-scf-runtime:

https://github.com/stevapple/swift-tencent-scf-runtime/blob/a60f1596247d7fa4d4cf83ae37e5965fc61275b2/Sources/TencentSCFEvents/APIResponse.swift#L61..#L69
https://github.com/stevapple/swift-tencent-scf-runtime/blob/a60f1596247d7fa4d4cf83ae37e5965fc61275b2/Sources/TencentSCFEvents/APIGateway.swift#L69..#L78

Motivation:
Request and Response are not handled in the same way. Request body is supposed to be of fixed type, thus using Generic. Response could have different types depending on processing result, thus using an enum to hold it.

@fabianfett fabianfett added this to the 1.0.0-alpha.1 milestone Aug 28, 2021
@fabianfett fabianfett added the ⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0 label Aug 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0
Projects
None yet
Development

No branches or pull requests

2 participants