Conversation
|
FML, I didn't realize these changes are completely incompatible with #3252 Will think about it more calmly |
|
ooh so sorry 🙈 |
|
@FBruzzesi I think if you were looking to clean things up following #3252, then #2486 may be what we're missing? |
| /, | ||
| *, | ||
| context: _LimitedContext, | ||
| schema: IntoSchema | None, |
There was a problem hiding this comment.
I think that if every one of these constructors is going to now require Schema, then we shouldn't return a dict from:
CompliantFrame.schema(and friends)
I understand that these changes make things cleaner inside the methods, but the classes as a whole are now less-ergonomic 🤔
There was a problem hiding this comment.
I'm bringing this up since from the perspective of plugins, this is an API break. on second read, that was a bit strong 🫣
We've already established that is expected for now, but I'd rather we get the consistency right in one swoop 🙂
There was a problem hiding this comment.
Ok that's fair! I will address the original issue and a bit more, but not too much else 😂
|
Closing in favor of #3266 |
3 participants
What type of PR is this? (check all applicable)
Related issues
typing.IntoSchematoo narrow #3257Checklist
If you have comments or can explain your changes, please do so below
Opening as draft - I will follow up with a few more tests