-
Notifications
You must be signed in to change notification settings - Fork 70
Thématisation de la page programme #1897
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
base: master
Are you sure you want to change the base?
Conversation
|
Si quelqu'un a une idée pour rector, je sais pas trop pourquoi il prend pas le .env correctement ? |
|
Pour résumer concernant .env, j'avais l'erreur suivante lors du lancement de rector dans la CI (uniquement): Cela était provoqué par la nouvelle entité EventTheme et son attribut: J'ai cru que le problème venait d'un mauvais chargement du fichier Du coup j'en ai profité pour ajouter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quelques derniers trucs, le reste est ok pour moi.
Y'a pas mal de tests behat qui ne passent pas, sûrement un soucis global.
| // Handle AJAX requests for updating data | ||
| if ($request->isXmlHttpRequest()) { | ||
| return $this->handleAjaxRequest($request, $event); | ||
| } elseif ($request->getMethod() === 'POST' && $request->request->has('delete')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Il y a un return dans le if au dessus donc pas besoin de else.
| } elseif ($request->getMethod() === 'POST' && $request->request->has('delete')) { | |
| } | |
| if ($request->getMethod() === 'POST' && $request->request->has('delete')) { |
ab0e620 to
f8a1bca
Compare
| port: "%database_port%" | ||
| user: "%database_user%" | ||
| password: "%database_password%" | ||
| port: "%env(int:DATABASE_PORT)%" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
est-ce que ce genre de changement technique ne serait pas à faire dans une PR dédiée pour ne déployer que ça, suivre le changement en prod, puis ensuite livrer en prod la fonctionnalité, mais bien dissocier les deux pour réduire les risques au déploiement / avoir un meilleur suivi) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comme la fonctionnalité ne s'active pas par défaut, on peut envisager de faire la mep globale sans activer la feature, pour ne rien casser en cas de rollback, qu'en dis-tu ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
poke @agallou ^^
|
@xavierleune Il y a toute une partie lié à la modification de la configuration. C'est pas le meilleur moment pour le moment pour changer un truc aussi sensible 😬 |
Cette feature est optionnelle: tant que l'option "Activer les thèmes" n'est pas cochée sur l'édition d'un event, les thèmes ne sont pas pris en compte à l'affichage. Cela permet de ne pas toucher au passé, mais également de travailler en 2 temps sur les futurs thèmes: mise en place du programme rapide puis ajout des thèmes (par exemple).
Important: la PR sur event doit être passée d'abord: afup/event#34