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

fix: fastify v3 compatibility #36

Merged
merged 6 commits into from Aug 20, 2020
Merged

fix: fastify v3 compatibility #36

merged 6 commits into from Aug 20, 2020

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Jun 12, 2020

For #35. Not including the test change that issue mentions as v3 is not stable yet, but I think this should work for both v2 and v3. I can confirm it works for v3 RC 4 (latest as of time of writing)

@puzpuzpuz puzpuzpuz added the enhancement New feature or request label Jun 12, 2020
@puzpuzpuz
Copy link
Owner

Thanks for submitting this PR! I'll leave it in "waiting for review" state until Fastify v3 is released. Once this happens, it would make sense to have tests for both v2 and v3.

@puzpuzpuz puzpuzpuz added this to the 2.3.0 milestone Aug 19, 2020
@puzpuzpuz puzpuzpuz self-requested a review August 19, 2020 17:18
@puzpuzpuz
Copy link
Owner

@SimenB as Fastify v3 is now released, we can proceed with this PR. Could you update tests, so that they run against Fastify v2 and v3? It should be similar to how it's done for koa v1, but we can reuse the same test script as in case of Fastify the API is the same. It would be also nice to update documentation, so it mentions both v2 and v3.

@SimenB
Copy link
Contributor Author

SimenB commented Aug 19, 2020

@puzpuzpuz updated. I removed the .use test for v3 since it requires some extra plugins for compat with the connect pattern

Copy link
Owner

@puzpuzpuz puzpuzpuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done. Left a couple of documentation related comments.

tests/fastifyv2.test.js Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
SimenB and others added 2 commits August 20, 2020 09:24
Co-authored-by: Andrey Pechkurov <37772591+puzpuzpuz@users.noreply.github.com>
@puzpuzpuz puzpuzpuz merged commit 6043c1f into puzpuzpuz:master Aug 20, 2020
@puzpuzpuz
Copy link
Owner

@SimenB many thanks for your contribution!

@SimenB SimenB deleted the patch-2 branch August 20, 2020 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants