Skip to content
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

Frontend Controller Refactor #5219

Merged
merged 1 commit into from
May 5, 2015
Merged

Conversation

acburdine
Copy link
Member

closes #5192, refs #5091

This PR refactors the frontend controller, combining the functions for the homepage, tag, and author routes into one function, with the intent of using it for channels in the future.

@acburdine
Copy link
Member Author

@ErisDS some of this code probably could be made better, but it works.

@ErisDS
Copy link
Member

ErisDS commented May 5, 2015

Really excited to see this! Such an enormous improvement to how the frontend works, test coverage for the controller automatically went up by ~10% due to the reduced duplication, and I think it's now significantly easier to read and manage 👍

The only thing I would prefer to be different is that the base URL/route for each channel be passed in as a whole rather than constructed inside the function, so it's passed in something like:

tag: renderChannel({
        name: 'tag',
        route: '/' + config.routeKeywords.tag + '/',
        ...
    }),

Or even using route: '/' + config.routeKeywords.tag + '/:slug/' and replacing the slug part. I realise the pagination part still needs to be appended depending on the case, but I think this moves us closer towards allowing users to customise the underlying routes for various things.

closes TryGhost#5192
- combines homepage, author, tag routes into one function (with different hash params)
- provides some abstraction for channels
@acburdine
Copy link
Member Author

@ErisDS fixed :)

ErisDS added a commit that referenced this pull request May 5, 2015
@ErisDS ErisDS merged commit 15b434d into TryGhost:master May 5, 2015
@acburdine acburdine deleted the controller-refactor branch May 9, 2015 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frontend controller refactor
2 participants