search

gce-electronics / HA_RFPlayer

RFPlayer plugin for Home assistant
Apache License 2.0
29 stars 10 forks source link

Crazymikefra/issue2 #25

Closed crazymikefra closed 1 year ago

prahal commented 1 year ago

@crazymikefra could you send separate MR for each functional change with an explanation of why they are needed (in each commit message body? That help keep a trace of why a change was made and if the code needs to be reworked helps a lot. Could you split them so the main branch still works if not all changes are applied?

(for the DoubleDom45 traces, you could include an extract from the trace in the commit message)

crazymikefra commented 1 year ago

@crazymikefra could you send separate MR for each functional change with an explanation of why they are needed (in each commit message body? That help keep a trace of why a change was made and if the code needs to be reworked helps a lot. Could you split them so the main branch still works if not all changes are applied?

(for the DoubleDom45 traces, you could include an extract from the trace in the commit message)

@prahal Bonjour. Ce push était une erreur de ma part. J'avais fait une fausse manip avec VSCode (un soir trop fatigué lol). Je suis en cours de refonte sur le fork de mon compte. J'ai utilisé une autre approche du sujet. Pour l'instant, je n'ai fait que partir de l'existant en le modifiant mais je prévois ensuite de repartir de zéro en respectant les bonnes pratiques de home assistant. Si tu veux en savoir plus, je te laisse regarder sur dépôt.

Bonne journée.

prahal commented 1 year ago

@crazymikefra je code dessus en ce moment mais il y a eds choses qui ne me plaisent pas dans send_command (https://github.com/gce-electronics/HA_RFPlayer/issues/23#issuecomment-1489628943). Ce problème n'est pas nouveau mais au lieu de le résoudre il y a des couches de code qui s'ajoutent autour et cela devient chaotique.

Sinon j'ai vu que tu ajoutes beaucoup de fonctionnalitéset travaille beaucoup le code. Mais les commits sont pas du tout segmentés. C'est chronophage de retrouver pourquoi un changement a été fait. Peut-être faudrait-il deux branches, une avec le code qui marche et l'autre avec l'ajout de code expérimental. Mais aussi c'est plus long de coder avec ces méthodes et il n'y a pas beaucoup de contributeur pour justifier le travail supplémentaire.

A propos j'ai vu un gros point noir en terme de bonne pratique. https://developers.home-assistant.io/docs/creating_platform_index "One Home Assistant rule is that the integration should never interface directly with devices. Instead, it should interact with a third-party Python 3 library. This way, Home Assistant can share code with the Python community and keep the project maintainable." rflb me semble tomber dans cette catégorie qui devrait être exportée dans une librairie python. Mais j'avoue que là je suis pas prêt à faire ce nettoyage de printemps.