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

Mise en place de Doctrine ORM #1700

Closed
wants to merge 1 commit into from
Closed

Conversation

Mopolo
Copy link
Contributor

@Mopolo Mopolo commented Mar 29, 2025

Comme discuté lors du dernier point mensuel, on avait décidé de tenter une transition vers Doctrine en mode ORM pour unifier les différents accès à la base de données.

Dans cette PR, il n'y a qu'une seule table de mappée avec l'ORM afin de simplifier la review et de s'assurer que tout fonctionne correctement en production avant de poursuivre avec d'autres tables.

@Mopolo Mopolo force-pushed the setup-doctrine-orm branch from c6bca62 to 04a7203 Compare March 29, 2025 15:38
@Mopolo Mopolo self-assigned this Mar 29, 2025
@Mopolo Mopolo force-pushed the setup-doctrine-orm branch from 04a7203 to 2a79737 Compare March 29, 2025 15:41
Comme discuté lors du dernier point mensuel, on avait décidé de tenter
une transition vers Doctrine en mode ORM pour unifier les différents
accès à la base de données.

Dans ce commit, il n'y a qu'une seule table de mappée avec l'ORM afin de
simplifier la review et de s'assurer que tout fonctionne correctement en
production avant de poursuivre avec d'autres tables.

Certaines options sont commentées car elles n'existent qu'à partir de la
v3 de `doctrine/orm`.
@Mopolo Mopolo force-pushed the setup-doctrine-orm branch from 2a79737 to 67db0b9 Compare March 29, 2025 15:45
@Mopolo Mopolo marked this pull request as ready for review March 29, 2025 16:02
@Mopolo Mopolo requested review from agallou and stakovicz March 29, 2025 16:02
@Mopolo
Copy link
Contributor Author

Mopolo commented Mar 29, 2025

@stakovicz @agallou Je pense qu'il faudra tester un deploy en staging au cas où.

@agallou
Copy link
Member

agallou commented Mar 29, 2025

au niveau de l'usage on avait parlé de potentiellement ne pas utiliser de DQL/requête à base proprités/faire seulement du SQL et possiblement de le vérifier via une règle PHPStan. J'avais commencé à plancher là dessus avec un début de règle comme cela : https://gist.github.com/agallou/c64a3c3c98cdc1556bd99cfd7c4b5f25, est-ce que ça vous irais ? Qu'en pensez vous d'intégrer ce genre de règles dès le début pour éviter de devoir faire le nettoyage après ?

doctrine:
dbal:
# url: '%env(resolve:DATABASE_URL)%'
url: 'mysql://%database_user%:%database_password%@%database_host%:%database_port%/%database_name%?charset=utf8mb4'
Copy link
Contributor

Choose a reason for hiding this comment

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

Il faut spécifier la timezone en query param, non ?

@@ -18,6 +18,8 @@
"cocur/slugify": "^2.3",
"cuyz/valinor": "^0.17.1",
"doctrine/dbal": "^2.5",
"doctrine/doctrine-bundle": "^2.7",
"doctrine/orm": "^2.20",
Copy link
Contributor

Choose a reason for hiding this comment

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

Peut être attendre l'arrivée de PHP 8.1 pour démarrer avec "doctrine/orm": "^3.0", non ?

@Mopolo
Copy link
Contributor Author

Mopolo commented Mar 31, 2025

au niveau de l'usage on avait parlé de potentiellement ne pas utiliser de DQL/requête à base proprités/faire seulement du SQL et possiblement de le vérifier via une règle PHPStan. J'avais commencé à plancher là dessus avec un début de règle comme cela : gist.github.com/agallou/c64a3c3c98cdc1556bd99cfd7c4b5f25, est-ce que ça vous irais ? Qu'en pensez vous d'intégrer ce genre de règles dès le début pour éviter de devoir faire le nettoyage après ?

Peut être attendre l'arrivée de PHP 8.1 pour démarrer avec "doctrine/orm": "^3.0", non ?

Alors j'ai du coup une autre proposition : #1705

@agallou @stakovicz on a le point outils mercredi prochain je vous propose qu'on reparle de ce sujet à ce moment-là.

@Mopolo Mopolo marked this pull request as draft April 7, 2025 09:22
@Mopolo
Copy link
Contributor Author

Mopolo commented Apr 14, 2025

Je cloture en attendant PHP 8.1

@Mopolo Mopolo closed this Apr 14, 2025
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.

3 participants