search

PerlDancer / Dancer2

Perl Dancer Next Generation (rewrite of Perl Dancer)
http://perldancer.org/
Other
537 stars 272 forks source link

Allow overriding of prefix in add_route. #1663 #1665

Closed GeekRuthie closed 11 months ago

GeekRuthie commented 2 years ago

The order of the arguments in add_route was such that, whenever you called add_route from a plugin, any 'prefix' entry you submitted was ignored utterly. Rearranging the order here lets plugin authors override the prefix on their add_route calls, even with undef.

xsawyerx commented 2 years ago

Before merging, the warnings in t/app.t need to be investigated.

GeekRuthie commented 11 months ago

I've pushed a commit that fills in some missing bits for the test state. These warnings were occurring when a 404 was reached, and the SERVER_PORT and SERVER_NAME were not set.

cromedome commented 11 months ago

Is there a reason a number of tests were commented out in t/app.t? The tests that changed to account for your patch make sense to me, but it seems like the ones that have been commented out are not related to the functionality in the branch.

Entirely possible I have missed something though :)

GeekRuthie commented 11 months ago

that was my goof. I shorted out a bunch of other tests while I was working on the failures @xsawyerx had pointed out, and flat forgot to re-enable them. Fix pushed to this branch.

cromedome commented 11 months ago

Approved :+1:

@xsawyerx I'm considering your approval to still be valid as the core code remains unchanged since when you approved it.

cromedome commented 11 months ago

Merged!

SysPete commented 11 months ago

closed via merge of 5b56729b56db447d8ac8fc98f196c1982345f6cc