-
Notifications
You must be signed in to change notification settings - Fork 274
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
Request uri_base method have to return URI object in all cases #1560
base: main
Are you sure you want to change the base?
Request uri_base method have to return URI object in all cases #1560
Conversation
that makes consistency better in cases of: - with the 'base' method, now both return URI objects - in various cases of '/' at end
Thanks for the PR @bor. I concur it would have been better if The refactor of @SysPete @cromedome The existing behaviour (and code) of string return from Should we consider another method that returns request uri_base as a URI object ? Something like the following would suffice.
|
I share @veryrusty's concern here. I am all for consistency... but not so much for breakage. Any suggestions for a method name? |
I've checked the documentation and didn't found any At the moment The documentation says |
Since current behaviour is clearly broken I'd like to propose that we mark |
Yeah, two similarly named methods sucketh (who suggested that?!). The After reflection, the fix in this PR is the way to go. However we need tests and documentation, and possibly a blog post somewhere about it. |
I can deal with the docs and blog post as part of the release if someone has time to cobble some tests together. I'll create an issue for creating a real deprecation policy, but for now, maybe go with two minor versions out and throw a warning, ie. "Method marked as deprecated and will be removed in Dancer2 0.500000". Thoughts? |
@xsawyerx and I spent some time reviewing this today. Our documentation says we return an object, but as @bor points out, we don't always. We're thinking we should approve and merge this, write the appropriate docs and blog post, and deal with the fallout. @veryrusty and @SysPete - penny for your thoughts? |
@veryrusty and @SysPete - penny for your thoughts? Thank you! |
The PR makes consistency better in cases of:
- with the 'base' method, now both return URI objects
- in various cases of '/' at end
The issue was that
=~
converts the value into a scalar in case of '/' at the end.