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

feat: Dimensions flat #36

Merged
merged 7 commits into from
Jun 10, 2022
Merged

feat: Dimensions flat #36

merged 7 commits into from
Jun 10, 2022

Conversation

goffle
Copy link
Contributor

@goffle goffle commented Jun 6, 2022

No description provided.

@goffle goffle requested a review from rap2hpoutre June 6, 2022 08:04
@goffle goffle requested a review from revolunet June 6, 2022 13:27
@revolunet
Copy link
Member

on devrait pas mettre jusqu'a 10 dimensions dans les inserts ? (vu qu'il y a 10 colonnes de prêtes)

@rap2hpoutre
Copy link
Member

See: #38

@lionelB
Copy link

lionelB commented Jun 7, 2022

Hello,
Est ce que vous avez prévu de gérer la migration des données existantes ?

@revolunet
Copy link
Member

je propose qu'on garde le comportement actuel avec le champ usercustomdimensions et on rajoute les nouvelles colonnes en plus ?

@lionelB
Copy link

lionelB commented Jun 7, 2022

je vais tenter un script de migration et si ça se complique, ça me va qu'on laisse la colonne usercustomdimensions le temps qu'on récupère suffisamment d'historique pour supprimer les ancienne colonnes)

@rap2hpoutre
Copy link
Member

rap2hpoutre commented Jun 9, 2022

Ok donc @lionelB tu reviens dans ce ticket pour nous dire si on peut la merge ? Ou alors on merge direct (cc @revolunet) et on effacera la colonne ensuite dans une deuxième PR si tu arrive à faire le script ?

EDIT:

En fait oui, je pense qu'on peut faire en deux temps comme ça on avance sur le sujet :

  • Etape 1 : on merge cette PR et à partir de maintenant tout va dans les nouvelles colonnes
  • Etape 2 : on développe un script de migration pour passer toutes les valeurs dans les nouvelles colonnes, car @goffle et @lionelB (au moins) en ont besoin pour leurs projets

Ça vous va ?

@lionelB Si tu n'as pas le temps je peux essayer de faire le script de migration (je découvre ce projet, mais j'imagine que ça devrait aller !) et vous faire une PR ?

@revolunet
Copy link
Member

revolunet commented Jun 9, 2022

est-ce qu'on peut pas garder l'insert dans usercustomdimensions pour eviter toute regression ?

comme ca on a juste une feature en plus : les dimensions flat.
on pourra supprimer la 1ere implem a un moment si ce n'est plus nécessaire ?

@lionelB
Copy link

lionelB commented Jun 9, 2022

je suis d'accord avec @revolunet et on pourra faire une pr pour le script de migration avec la suppression des ancienne colonnes

@rap2hpoutre
Copy link
Member

Parfait, merci faisons comme ça. @revolunet j'ai fait la modif (via une PR comme j'ai pas les droits) pour garder les deux fonctionnement et @goffle vient de la merge. Si c'est bon tu peux merge @revolunet.

Side question : @lionelB tu veux faire le script ou je le fais alors ?

@goffle goffle merged commit efe505d into master Jun 10, 2022
@goffle goffle deleted the feat--dimension-flat branch June 10, 2022 10:43
@github-actions
Copy link

🎉 This PR is included in version 1.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants