This repository was archived by the owner on Jun 21, 2022. It is now read-only.
Refactor controller names (please review, but don’t merge yet)#137
Open
Refactor controller names (please review, but don’t merge yet)#137
Conversation
for c in Article Dish Item Meal Permission Print PurchaseList Quantity Recipe ShopSection Tag Unit
do
echo "s/Controller::$c/Controller::Project::$c/;"
done \
| sed -f- -i $( git ls-files lib/ t/ )
for c in article dish item meal permission print purchase_list quantity recipe shop_section tag unit
do
perl -pe 's{ \W \K /'$c' }{/project/'$c'}x' -i $( git ls-files lib/ )
done
Never got @CARP_NOT to work like I expected. %Carp::Internal is not as internal as it sounds.
for c in article dish item meal permission print purchase_list quantity recipe shop_section tag unit
do
perl -pe 's{ action \s* => \s* \K '\'$c"}{'project/"$c'}x' -i $( git ls-files lib/Coocook/Controller/ )
done
for c in article dish item meal permission print purchase_list quantity recipe shop_section tag unit
do
git mv -v root/templates/{,project/}$c/
done
for c in article dish item meal permission print purchase_list quantity recipe shop_section tag unit
do
perl -pe 's{ [^\w/] \K '$c' (?= [\w/]+ \.tt ) }{project/'$c'}x' -i $( git ls-files lib/ root/templates/ )
done
Owner
Author
|
ping @moseschmiedel |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I had a quick idea to resort the modules below Coocook::Controller. Please review: Primarily is that an enhancement? Technical review might not be that important because tests pass and I am pretty sure it’s complete. Please see below for open question.
The thing is our controllers which handle project-specific data were spread all over the controller namespace. Some of them would chain from
/project/baseand others not.Before
Can you spot the project-specific controllers?
After
Open question
In classic Catalyst logic we have
Project.pmandProject/with submodules next to each other. This is all project-specific.Then the are the
Browsemodules like I named them. There are not about 1 project but exploring all Coocook data. That is:Current state of this PR
Solution 0:Project::ProjectcontrollerThat name reads like a mistake.
Solution 1:
RootcontrollerThis is also what Catalyst does for the application’s main controller
MyApp::Controller::Root.Solution 2: explicit namespace for project-specific controllers
Optional next step: dissolve
Controller::BrowsenamespaceDid you ever dislike the
Browsenamespace? It’s pretty generic andPOSTactions are not like “browsing”.