-
Notifications
You must be signed in to change notification settings - Fork 29
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 à jour de phoenix_live_view de 0.19 à 0.20 #3956
Conversation
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/phoenix@1.7.11, npm/phoenix_live_view@0.19.5 |
@@ -24,7 +24,7 @@ $tooltip-width: 150px !default; | |||
z-index: 1; | |||
bottom: 125%; | |||
left: 50%; | |||
margin-left: -($tooltip-width / 2); | |||
margin-left: -(calc($tooltip-width / 2)); |
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.
👍
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.
C'était nécessaire comme changement ? Il me semblait que ça fonctionnait sans.
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.
@AntoineAugusti ça faisait des warnings dans les logs.
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.
(Rien à voir avec la mise à jour de Liveview, c’est juste que du coup j’étais attentif à ce qui passait dans les logs et que je l’ai vu.)
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.
LGTM
Navré pour le calc manquant, c'était une suggestion non testée de ma part lors d'une review.
Note: pour calculer le changelog, tu peux utiliser ceci: Changelogs(initially generated with
|
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.
Je recommande qu'on déploie sur prochainement
et surtout qu'on regarde au niveau logging ce qui s'y passe (ex: éventuels deprecation warnings qui resteraient au runtime au chargement d'une LiveView, par exemple). Récemment il y a eu un peu de "casse" dans le sens où il peut y avoir des runtime logs qui vont aller se coller dans AppSignal / CleverCloud, générer de l'IO et manger notre quota de logs (payant).
J’ai comparé les logs de prod et de prochainement pour des sessions similaires avec pas mal de Liveview, c’est similaire, y’a les mêmes lignes. Rien de marquant dans les logs de déploiement. On déploie en production. |
Fixes #3831
Le
mix deps.get
a causé la mise à jour de plus de librairies que prévu (dont phoenix, plug et cowboy) mais rien de notoire dans le changelog.RAS, j’ai vérifié les déprécations et warnings, on utilisait aucune des fonctions concernées.
J’ai fait un tour en local sur les bouts de liveview (discussions, validations, formulaire de feedback, notifications, backoffice), ça se comporte comme ça devrait.
J’ai aussi réglé un warning SCSS qui n’avait rien à voir.
Marche à suivre pour la prochaine fois
mix deps.get
cd apps/transport/client/
yarn upgrade phoenix_live_view