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

Match function #2142

Merged
merged 1 commit into from Dec 11, 2022
Merged

Match function #2142

merged 1 commit into from Dec 11, 2022

Conversation

pjebs
Copy link
Contributor

@pjebs pjebs commented Oct 6, 2022

@welcome
Copy link

welcome bot commented Oct 6, 2022

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

@li-jin-gou
Copy link
Contributor

li-jin-gou commented Oct 7, 2022

Please follow the PR template @pjebs
https://github.com/gofiber/fiber/blob/master/.github/pull_request_template.md

@efectn
Copy link
Member

efectn commented Oct 7, 2022

Can you add some benchmarks

@ReneWerner87
Copy link
Member

ReneWerner87 commented Oct 10, 2022

  • add some benchmarks

@pjebs
Copy link
Contributor Author

pjebs commented Oct 10, 2022

What should it benchmark?
What input values should go into the function?

@ReneWerner87
Copy link
Member

ReneWerner87 commented Oct 10, 2022

What should it benchmark?

the speed of the process, so that we can gradually optimize it to always offer the fastest possible solution to our framework users

What input values should go into the function?

an example of how it would also be used normally

@efectn
Copy link
Member

efectn commented Nov 13, 2022

Can you check last comments @pjebs

@ReneWerner87
Copy link
Member

@pjebs friendly reminder
we need the documentation part and some benchmarks so we can optimize this functionality in the future

@pjebs
Copy link
Contributor Author

pjebs commented Nov 21, 2022

This PR is now compete.
I don't think there is any need for documentation on the website. godoc documentation should be adequate.
Benchmark has been added.

path_test.go Outdated Show resolved Hide resolved
@ReneWerner87 ReneWerner87 linked an issue Nov 21, 2022 that may be closed by this pull request
3 tasks
@ReneWerner87
Copy link
Member

ReneWerner87 commented Nov 21, 2022

I don't think there is any need for documentation on the website. godoc documentation should be adequate.

all features we provide to the user are explained in our markdown documentation with examples and further information, why should we break with that for this feature ?

@pjebs
Copy link
Contributor Author

pjebs commented Nov 21, 2022

@ReneWerner87 Where in docs should this function be documented? I don't see a good section for it.

@ReneWerner87
Copy link
Member

@ReneWerner87 Where in docs should this function be documented? I don't see a good section for it.

Good point
normally we would need another section which utils which are usable for all documented

since we don't have this and I am okay with your statement that the godocs are currently sufficient

@pjebs
Copy link
Contributor Author

pjebs commented Dec 11, 2022

Can this be merged and tagged?

@efectn
Copy link
Member

efectn commented Dec 11, 2022

Can this be merged and tagged?

Can you create a PR for docs repo? Route category may be nice for this feature.

@ReneWerner87 ReneWerner87 merged commit f13c948 into gofiber:master Dec 11, 2022
@pjebs pjebs deleted the add-Match-func branch December 11, 2022 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 [Feature]: Export matching function
5 participants