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

On enlève SiteBaseController et tout ce qui va avec #1605

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stakovicz
Copy link
Contributor

On enlève SiteBaseController.

Création du service WebsiteBlocks qui permet de construire les blocks et de faciliter l'injection aux templates via la fonction render().

*
* @return Response A Response instance
*/
public function render(string $view, array $parameters = [], Response $response = null): Response
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On peut discuter du nommage du service et de cette fonction, pour que ce soit bien clair.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm on a un namespace pour twig donc je propose un truc de ce genre :

  • \AppBundle\Twig\Renderer
  • \AppBundle\Twig\TemplateRenderer
  • \AppBundle\Twig\ViewRenderer

Et le nom de la méthode me semble bien.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bien vu pour le namespace.
J'aime bien : \AppBundle\Twig\ViewRenderer

@stakovicz stakovicz marked this pull request as ready for review January 28, 2025 06:32
@stakovicz stakovicz self-assigned this Jan 28, 2025
@stakovicz stakovicz requested review from agallou and Mopolo January 28, 2025 06:32
*
* @return Response A Response instance
*/
public function render(string $view, array $parameters = [], Response $response = null): Response
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm on a un namespace pour twig donc je propose un truc de ce genre :

  • \AppBundle\Twig\Renderer
  • \AppBundle\Twig\TemplateRenderer
  • \AppBundle\Twig\ViewRenderer

Et le nom de la méthode me semble bien.

*
* @return Response A Response instance
*/
public function render(string $view, array $parameters = [], Response $response = null): Response
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Une question, j'ai checkout la branche et je ne trouve au appel qui passe une $response. Est-ce qu'il y en a que je n'ai pas trouvés ?

Et sinon, est-ce que c'est nécessaire de garder ce paramètre ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Effectivement, j'ai repris la même signature que la fonction render() fourni par le Controller, mais ce n'est pas utile.

@stakovicz stakovicz force-pushed the remove-site-base-controller branch 2 times, most recently from 0a4f112 to 2bc244b Compare January 29, 2025 19:15
@stakovicz stakovicz force-pushed the remove-site-base-controller branch from 2bc244b to 17a7e60 Compare January 29, 2025 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants