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: use case when route defined without a url #3049

Merged
merged 2 commits into from
Apr 30, 2021

Conversation

amitgilad3
Copy link
Contributor

@amitgilad3 amitgilad3 commented Apr 29, 2021

Checklist

This merge is intended to fix the bug #3047.

in case of empty url in route - throw error

@amitgilad3 amitgilad3 changed the title handle use case when route defined without a url Fix: use case when route defined without a url Apr 29, 2021
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm


const fastify = Fastify()
try {
await fastify.route({
Copy link
Member

Choose a reason for hiding this comment

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

can you add a test for shorthand declaration as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@zekth zekth left a comment

Choose a reason for hiding this comment

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

lgtm

@Eomm Eomm added the bugfix Issue or PR that should land as semver patch label Apr 30, 2021
Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

Closes #3047

Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

LGTM.

@mcollina mcollina merged commit eff4e38 into fastify:main Apr 30, 2021
@mcollina
Copy link
Member

Thanks!

@amitgilad3
Copy link
Contributor Author

really enjoyed resolving this issue, hope to contribute more in the future

@github-actions
Copy link

github-actions bot commented May 1, 2022

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 May 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bugfix Issue or PR that should land as semver patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants