-
Notifications
You must be signed in to change notification settings - Fork 99
Support collection types as a top level object #125
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
Support collection types as a top level object #125
Conversation
|
refers to this issue #119 |
AvdLee
left a comment
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.
Thanks, this makes sense! Can you update the tests as well to verify this is working as expected?
BasThomas
left a comment
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.
This is awesome!
raphkoebraam
left a comment
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.
🚀
|
@batuhansk could you pull latest |
sure, done! |
SwiftLint found issues
Code Coverage Report
Generated by 🚫 Danger Swift against accd9f5 |
|
Congratulations! 🎉 This was released as part of Release 2.7.0 🚀 Generated by GitBuddy |
rogerluan
left a comment
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.
This PR made release v2.7.0 break the contract (APIs), thus who updated from 2.6.0, this change broke their envs. Was this expected? Shouldn't this have been a major version bump (e.g. v3.0.0)? 😬 @batuhansk @AvdLee
Ah, you're right! That went unnoticed, mostly since I think I thought Did you run into many build errors? |
|
Yes, our tests started failing upon upgrading from v2.6.0 to v2.7.0, so for now we locked on v2.6.0 (
Whatever way we decide, we have to inform users how to migrate from one to another I think (regardless if they're doing this in v2.7.0, v2.8.0 or v3.0.0). This can be done via a e.g. WDYT @AvdLee ? |
|
Could you show me the errors you got? Would be great if we can open a branch with that error reproduced. That way, I could play around with the API and make the code work for both backwards compatibility, as well for @batuhansk's case |
|
I don't have the build logs anymore and would take some time to reproduce the environment in which this occurs, but the error was here: mock.onRequest = { _, body in
guard let body = body else { return }
XCTAssertEqual(body["answers"] as? [String], answers)
parametersExpectation.fulfill()
}
So the error was like "body of type Any doesn't have a subscript etc…" something along those lines :) Does this help @AvdLee ? 🙇 |
|
@rogerluan makes sense! I've opened an issue here: #132 I can't promise any timelines here, but feel free to jump on it if you have the time! |
|
Congratulations! 🎉 This was released as part of Release 3.0.0 🚀 Generated by GitBuddy |
As you know, top level collection objects are also valid JSON. But, Mocker wasn't support them because of the casting that made up in the
MockingURLProtocol.swift