On 20 Feb 2009, at 22:57, Daniel Westermann-Clark wrote:
On 2009-02-11 21:53:48 +0000, Tomas Doran wrote:
Why not just add a remote_user() method on $c->req instead? It's a
little more typing, but is more explicit about where the value comes
from and doesn't potentially break any existing apps.
Patches on 5.80 welcome :)
Attached is a set of patches to add support for $c->req->remote_user,
including a basic test.
Good stuff, thanks. I've branched 5.80 trunk and applied your Runtime
change, and then I've fiddled the 'do we warn' logic to be a bit
safer. Have a look and let me know what you think?
http://dev.catalystframework.org/repos/Catalyst/Catalyst-Runtime/5.80/
branches/deprecate_req_user/
Test for the new functionality looks fine.
There is also a deprecation warning for non-Catalyst packages calling
$c->req->user.
Is anything in the current test suite triggering the new warning? If
so, can you switch it over to be calling ->remote_user instead, and
can you add a call to read ->user which provokes the warning, and
test you get the expected warning (see t/deprecated.t r9354 - you
could just add the warning test here/to that app which already has
its global logger overridden?)
I'm not sure about the engine patches, but it seemed like a
forward-compatible way to add support now for the new method.
They look totally sane to me. Lets get Runtime right and a nod from
andyg before applying them however :)
Feel free to supply another patch against the branch, or hop on irc
and grab a commit bit so you can commit yourself?
Cheers
t0m
_______________________________________________
List: [email protected]
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/[email protected]/
Dev site: http://dev.catalyst.perl.org/
