declareNamedSchema takes a concrete Proxy #180
Conversation
src/Data/Swagger/Internal/Schema.hs
Outdated
| declareNamedSchema = genericDeclareNamedSchema defaultSchemaOptions | ||
|
|
||
| -- | Convert a type into a schema and declare all used schema definitions. | ||
| declareSchema :: ToSchema a => proxy a -> Declare (Definitions Schema) Schema | ||
| declareSchema = fmap _namedSchemaSchema . declareNamedSchema | ||
| declareSchema :: forall a proxy. ToSchema a => proxy a -> Declare (Definitions Schema) Schema |
There was a problem hiding this comment.
Or should this functions use concrete Proxy too? I'm not sure.
- The class member type-signature change is already a breaking change, so we can break more
- is there value in consistency?
I'm quite sure I used some of of swagger2 combinators with not-Proxy, but it was indeed more of "cleverness" than strict requirement (i.e. look, I actually have Maybe a value here, I can plug).
There was a problem hiding this comment.
That's a good point.
Changing them all has the advantage that nobody will be confused about this difference, even if it otherwise doesn't matter in any significant way.
There was a problem hiding this comment.
I think it makes sense to switch to Proxy everywhere eventually. And since we are doing a breaking change, why not break all of it in the same way? 😊
|
Alright @phadej, I believe this is now as you wanted. |
|
Thanks! |
Fixes #150
As mentioned on the issue, this would be especially valuable for enabling
ToSchemato be derived withDerivingVia.This would be a breaking change, but hopefully the actual incurred code updates would be minimal. Happily the code changes are quite small, and all the other functions besides
declareNamedSchemacontinue to accept polymorphicproxys.