-
Notifications
You must be signed in to change notification settings - Fork 19
Use new beforeEach signature in TestPrinters adapter #186
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
Conversation
|
PR is merged. Please let me know when you're ready to push a preview release so we can coordinate. |
|
I don't think I have the infra to produce preview releases atm I think 😓. That being said, support for the new .NET testing platform just got merged, so combining those two into a breaking change might be a good way forward. |
|
We've got some more breaking changes queued up on the Expecto side, so 11.x might not reach general audiences for a bit. Looks like you're using Release Please? |
I am. Never had to use it to do pre-releases before though. I think it can be done by crafting a commit with a specific message, but it's not something I've ever tried. That being said, nuget packages are created on every build, and could be uploaded to gh as artifacts; so it should be possible to download one of them and just drop them in a local nuget-store for testing. |
|
I've been thinking, and I think requiring a coordinated version change is likely to cause trouble. For example, what if we make a multi-phase migration like this Phase 1: Add a builder method or some other way consumers can create a TestPrinter that isolates them from change in the printer signature Expecto.Impl.TestPrinters.silent
|> TestPrinters.withBeforeEach newBeforeEach
//...Deploy that change in some Expecto 10.x release Phase 2: YoloDev updates to use the builder method Phase 3: Expecto update the printer type, using the builder method to map the old signature into the new one without disrupting callers of the builder method Thoughts? |
|
This wouldn't get us compatibility between all versions of Expecto and the YoloDev adapter, but it should reduce breakage going forward and also reduce the likelihood a consumer runs into the incompatibility |
|
Here's the Expecto PR to add those builder methods (on the Expecto side) Once we decide on an approach, I'd be happy to push an Expecto update and make a PR here too |
That seems like a much better idea. If we can release a non-breaking version of Once 11 is out, we might also try to see if we can have tests in this repo against both 10 and 11 for a while. |
|
@farlee2121 what's the latest? Is this still in progress? etc. |
|
I think PR #210 probably makes this unnecessary. Since the isSkipped parameter isn't used by this project, and this project is now using the builder methods, it shouldn't need to change anything else to work with Expecto 11.0.0. |
|
So I just close this then? |
|
@rynoV Do you have any outstanding concerns? |
|
Looks like #210 handles this nicely, thanks folks |
A small update for compatibility with changes from haf/expecto#516
I believe this will need Expecto
11.0.0-alpha6but will confirm when the PR is merged