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

Update to avvio@7.0.0-alpha.0 #2093

Merged
merged 2 commits into from
Feb 15, 2020
Merged

Update to avvio@7.0.0-alpha.0 #2093

merged 2 commits into from
Feb 15, 2020

Conversation

mcollina
Copy link
Member

Update to avvio@7, implementing await app.register() and await app.after().

Checklist

  • run npm run test and npm run benchmark
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message and code follows Code of conduct

Copy link
Member

@jsumners jsumners left a comment

Choose a reason for hiding this comment

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

Will there be a "final" release before v3? Maybe we should have an automated audit during a release stage to prohibit pre-release modules in the dependencies and warn for non-semver major ones?

@mcollina
Copy link
Member Author

@jsumners I think we should plan the release for v3 with some caution. I'm thinking to have some -alpha releases and a few release candidates. We can then update the modules. I would lean to do a semver-major change everywhere because I think we should drop old Node.js versions anyway.

@jsumners
Copy link
Member

I agree. I just want to make sure we don't release a final v3 with release candidate tagged dependencies.

Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

I would write more extensive documentation and add some more test, we are not covering await fastify.register at the moment.

docs/Server.md Outdated
Comment on lines 436 to 437
console.log('After current plugin')
})
Copy link
Member

Choose a reason for hiding this comment

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

Leftover :)

@delvedor delvedor added semver-major Issue or PR that should land as semver major v3.x Issue or pr related to Fastify v3 labels Feb 13, 2020
@delvedor delvedor added this to the v3.0.0 milestone Feb 13, 2020
@mcollina
Copy link
Member Author

I've added a test for await app.register() and found a non-blocking bug.

@delvedor which kind of doc would you like to see? I struggled a bit in finding a good place where to add more.

I would like to land this ASAP so we can unblock all the other PRs.

Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

LGTM

Let's open an issue for this bug and another one for improving the documentation.
People will likely use async-await a lot, and we should do better work on showing examples of how to use it.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
semver-major Issue or PR that should land as semver major v3.x Issue or pr related to Fastify v3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants