-
Notifications
You must be signed in to change notification settings - Fork 8k
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
Hold matched route full path in the Context #1826
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1826 +/- ##
==========================================
+ Coverage 98.74% 98.74% +<.01%
==========================================
Files 38 38
Lines 2143 2157 +14
==========================================
+ Hits 2116 2130 +14
Misses 15 15
Partials 12 12
Continue to review full report at Codecov.
|
Oh, I wasn't aware of multiple previous attempts... |
The pull request benchmark data:
The master branch benchmark data:
|
About the proposal, #1390 committed by @4396 (thanks!) on 12 Jun 2018 first, and #1447 committed by @youngbloood (thanks!) on 28 Jul 2018, but they have a bit performance issue, please see #1390 (comment) and #1447 (comment). |
I am fine with anything as long as the changes land on master. |
@zaynetro Could you also update the documentation? |
I wasn't sure where to update the documentation so decided to extend an existing example. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks @zaynetro
Nice feature!, could you release a new version with this feature?, I wanna use it without lock with the master branch. |
@thinkerou @appleboy @javierprovecho Can you release this useful feature that we can properly version this module, please? |
@Dominik-K yes |
* Return nodeValue from getValue method * Hold route full path in the Context * Add small example
@thinkerou nice feature, but why is it labeled |
This is a proposed change to return handler full path from the Context.
Where it can be useful?
When using lambda functions as a handler names returned from
(*Context).HandlerName
become not lean. With this change it becomes easy to analyse which route was called exactly. This can be useful when trying to find out which routes are not being called in the app.Alternatives
It is possible to create a wrapper around route definition:
This approach is doable but considering that Gin already holds full path in its structures I though that it might be better to just expose it.
P.S. README no longer contains any docs so I am not certain where I should document this feature.