-
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
prettify error message for catch-all conflict with existing path segment #2934
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2934 +/- ##
=======================================
Coverage 98.57% 98.57%
=======================================
Files 41 41
Lines 3080 3085 +5
=======================================
+ Hits 3036 3041 +5
Misses 31 31
Partials 13 13
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@appleboy @thinkerou Hi, sorry to bother, any suggestions about this change and the diff coverage? I have no idea why my changes will remove coverage for |
Try to merge from |
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.
You may need to add some unit test cases for this change, too.
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
Hi, I find that the conflict message for catch-all wildcard with existing path segment is not clear enough during my use of gin framework, i.e. it only suggests that there is a conflict, but gives no hint about where the conflict is.
This PR will give a more user-friendly error message when users are trying to add two conflict routes as bellow:
Error message before:
Error message now: