Service Netatmo

Ok top, merci pour la réponse.

Puis-je te demander juste de faire un saut rapide sur la nouvelle PR : [WIP] - Netatmo add valves NRV by Terdious · Pull Request #2005 · GladysAssistant/Gladys · GitHub pour me confirmer que c’est bon ?
Je voudrais être sûr de la méthodologie quand on crée une PR depuis une autre branche et de l’impact des historique de commit en sachant que dans cette nouvelle PR on voit l’historique de l’autre PR. Et qu’à la création de la nouvelle PR cela à relancé les tests sur la précédente … toujours un peu inquiétant quand on ne fait pas ça souvent ^^

Merci par avance !

Je pense que tu as du te tromper, parce que je vois exactement le même nombre de commits dans une PR comme dans l’autre, et avec exactement les mêmes commits

Bonjour @pierre-gilles !
Oui alors en partant de la facon que j ai fait, c’est normal, pour le moment je n ai rien push, j ai fait en local.
Justement d’où ma question. Je suis parti de ma branche de base netatmo-integration pour creer ma nouvelle branche basée sur origin/gladys (pour partir sur le code de cette 1ere PR et commencer a integrer la suite). J ai juste push la nouvelle branche puis creer la nouvelle PR liée au repo GladysAssistant.
Est-ce bien la bonne méthode pour être clean ?

Ah ok, dans ce cas c’est tout bon :slight_smile: Pense juste à commit dans la bonne branche

1 Like

Super, merci pour la réponse confirmation ^^

Oui pour les push, ca va apres toutes les betises que j’ai pu faire sur mon projet perso, je suis meilleur de ce côté :sweat_smile:

1 Like

Nouvelle PR prête ajoutant les vannes NRV :

Image de test en cours de build : docker pull terdious/gladys:netatmo-add-valves

Je poursuis avec la station météo.

4 Likes

On ne t’arrête plus…

1 Like

Nouvelle PR prête ajoutant les bases stations météo intérieurs :
[WIP] Add support for NAMain module type - Weather / Smart Home Weather Station #2010

Image de test en cours de build : docker pull terdious/gladys:netatmo-add-weather-station

Je vais poursuivre avec les modules de la station météo un par un.

:sweat_smile: en espérant que la 1ère PR est suffisamment bonne tout de même ^^ mais normalement j’ai attaché les branches pour que les mises à jour soient simplifiées. Ca ne devrait pas trop impacter les autres PR en cas d’évolutions demandées dans la review !!

Edit :

@PhilippeMA, je te préviens quand j’ai terminé la station complète pour que tu puisses tester si toujours OK !

3 Likes

Nouvelles PR prête ajoutant :

Image de test en cours de build intégrant la totalité des PR en cours Netatmo : docker pull terdious/gladys:netatmo-add-module-outdoor-weather-station


image

@Terdious Review faite sur la première PR ! C’est beaucoup mieux :slight_smile:

J’ai quelques feedbacks mais rien de compliqué !

2 Likes

Merci beaucoup @pierre-gilles !

Les correctifs sont fait !! Je les reporte sur les autres PR.

Merci pour les retours ! Pour la concurrence, je recommanderais de rester à max 2 pour des trucs qui tournent en background comme ça. Il y a des utilisateurs sur des Pi, on veut rester un logiciel léger et éviter les gros spikes :slight_smile:

Tiens moi au courant dès que la doc est à jour. Re-teste bien tout pour vérifier que tu n’as rien cassé en faisant les correctifs, et pour moi on pourra faire une mise en prod de cette première version :slight_smile:

1 Like

C’est tout bon pour moi @pierre-gilles,

  • Concurrence fixée à 2 : :white_check_mark:
  • Nouveaux tests complet sur nouvelle installation suite aux fix de la review: :white_check_mark:
  • Documentation mise à jour avec les images actuelles (en et fr) : :white_check_mark:
  • Ajout d’une vue du Dashboard en conclusion : :white_check_mark:
  • Image docker terdious/gladys:netatmo-integration-dev à jour au besoin : :white_check_mark:

Edit : Les 4 PR suivantes sont prêtes à review avec les maj de la 1ère. Tu pourras t’y intéresser dès que tu as du temps mais pour info elles sont vraiment très légères tu verras. Je n’ai pour le moment mis que ce qui est existant (sauf pour la 1ère de la station météo qui nécessitait 1 ou 2 constantes supplémentaire mais rien de méchant).

  • Image docker terdious/gladys:netatmo-add-module-outdoor-weather-station pour ceux qui le souhaite : :white_check_mark:
  • Pluviomètre (simple) et Anémomètre (un peu plus difficile - ajout front dashboard) en cours :eight_pointed_black_star:
1 Like

Bon forcément il a fallu que codecov crash sur le dernier push (je ne peux toujours pas les relancer), mais les tests passent bien en local je te rassure :

Edit : Bon c’est moche mais mon seul moyen pour les repasser c’est de faire un double « fake push »…
Ca se produit clairement pour ma part à chaque fois qu’il y a au moins 2 PR en cours de tests. Hier cela c’est produit 3 fois lorsque je réalisais des push sur différentes PR en peu de temps.

2 Likes

Excellent :sunglasses:

Merci pour ta réactivité !!

C’est bon pour moi, c’est accepté sur Github et mergé sur master :rocket:

Pour la doc, c’est très propre !

Je mergerais la doc au moment de la release (sûrement semaine pro)

t’as bien raison :smiley: c’est pas un souci d’avoir des commits sales, vu que c’est juste dans la PR, je merge en squash ensuite moi

3 Likes

Finalement je suis revenu un peu en arrière, j’ai refais les branches et PR à partir du master à jour. C’était trop galère à mettre à jour et c’est beaucoup plus propre.
J’ai donc 2 PR prête à review :

Testées entièrement :

2 Likes

On avance sur la station météo :

Pluviomètre et on est bon pour passer à la suite !

4 Likes

Tout bon pour la station météo :

@pierre-gilles tu pourras review les 6 PR (je les ai faites indépendantes mais l’ordre est important au moins pour les PR 1 et 2).
Et tu me diras si tu préfères, j’ai une branche qui intègre la totalité de ces 6 équipements. Ca peut faciliter les petits correctifs apportés au fur et à mesure et surtout la review (46 fichiers sur la branche unique contre 6 PRx20aine de fichiers en découpage puisque la plupart des fichiers sont impactés dans chaque PR).

@PhilippeMA, si tu es motivé pour tester la station météo au complet : docker pull terdious/gladys:netatmo-features-nrv-weather

3 Likes

Ok génial :slight_smile: lundi je vais faire une release je pense, je release déjà la première version

Ensuite, je review le reste et ça partira dans la version d’après. Ça te paraît bien ?

Merci pour ta réactivité en tout cas c’est vraiment chouette toute ces PRs !!

1 Like

Super !! Ca me parait très bien ^^
Je vais mettre la PR regroupant les 1 à 6 en plus (Grouping of PRs 1 to 6: Addition of complete Energy and Weather functionality by Terdious · Pull Request #2021 · GladysAssistant/Gladys · GitHub). Comme ça tu pourras voir ce qui est le plus intéressant à review / merge.

C’est un plaisir !! Merci pour ton retour !

1 Like