[cgiapp] RE: Patches available for ::Plugin::Routes (was: Re: New plugin idea based/inspired on CA Dispatch)

[Porta]

Wonderful changes.
I'll review the POD problem. First time with it.

Thanks.

Julián.


> I already did the change you suggested to change "routes_table" to
> "routes".

Great.

> with this one.
>
> - Consider having "routes_table()" automatically add every run mode
> dispatched to
>  the "run_modes" hash, if a subroutine with that name exists in the current
>  scope. (I suppose you can use the "can()" method to test for that? ).
>
> Maybe because I'm not pro-efficient with perl enough, or maybe I'm
> missing something?

I've sent you patches via github that implement this for you now. You
can browse them here:

http://github.com/markstos/cgi--application--plugin--routes/commits/master

When registering routes, we also try to automatically register the run
mode names. We will register a run mode when there is not already one
defined with the target name and we 'can' call a method with the same
name.

This means that for you common cases when you call 'routes', you can
skip calling run_modes().

My other patches improved your 'import' routine, created proper
automated tests, and made sure Data::Dumper was only loaded if it was
required.

> Also,. I liked this:
>
> Perhaps there should be an option to set a path for which all the routes are
> relative to. For example:
>
>  $c->routes_root('/app/volunteer/profile');
>
> I definitely going to include that change.

Great!

> But, about the catalyst style, I didn't liked the approach, and, also,
> see something (I think almost) exactly like that in
> CAP::Actiondispatch
> (http://search.cpan.org/~jaywhy/CGI-Application-Plugin-ActionDispatch-0.96/lib/CGI/Application/Plugin/ActionDispatch.pm)
> and I really didn't liked it (or just didn't worked for me).

I had forgetten about that. I never used it myself. I think I prefer the
approach in this new plugin.

A bit of further feedback:

- Your POD coverage test is still failing.

    Mark

-- 
http://mark.stosberg.com/