WIP: Resolve "counter test module"
Closes #117 (closed)
Merge request reports
Activity
!105 (ddc5e779) -> le type de commit ne conviens pas, ce n'est pas un refactor mais l'ajout d'un nouveau module, ce commit devrai plutôt se nommer
[feat] create counter module. De plus, ce commit n'est qu'un copier coller direct du module skeleton, ça ne vas pas, il faut au minimum renomer le module avant de commiter :)!105 (245e1210) -> ce commit est a fusionner (squasher) avec le précédent.
!105 (59d2e9cd) -> le nom et type de commit ne conviennent pas. C'est un commit ou tu relie ton module au binaire durs-server, on nomp de commit correct serait plutot
[feat] counter-mod: add counter mod to durs-serverSur ce dernier commit toujours : on ne commit jamais de commentaire contenant des questions ! Si tu a une question surt un e ligne précise du code il faut créer la question sur gitlab, si tu vas voir les diff de la MR sur cette présente page tu verra que tu peut ajouter un commmentaire sur une ligne précise.
Général : la CI ne passe pas : https://git.duniter.org/nodes/rust/duniter-rs/-/jobs/16821
Tu a oublié de lancer fmt avant de commiter ou/et ton fmt n'est pas a jours. Pour mettre a jours fmt (et rust dans la foulée) :
rustup updateEdited by Éloïs33 33 #[cfg(unix)] 34 34 pub use durs_tui::TuiModule; 35 35 //pub use durs_skeleton::SkeletonModule; 36 pub use durs_counter::CounterModule;//ais-je bien défini durs_counter? -
On n'écris jamais de question a destination d'autres dev directement dans le code, pour poser une question sur une ligné précise du code ça se passe sur le gitlab dans la MR onglet "Changes".
Réponse a ta question : il me semble que oui :)
Edited by Éloïs -
@counter-reverse ok si c'était une note pour toi dans ce cas la convention est de commencer le commentaire par
// TODO, ainsi avant de commiter tu peut chercher tout les TODO avec le moteur de recherche de ton IDE et les supprimer :) changed this line in version 3 of the diff
added 5 commits
-
59d2e9cd...08bdcde4 - 2 commits from branch
dev - 3c56f45f - [feat] create counter module
- ff084369 - [feat] counter-mod: add counter mod to durs-server
- e108dd18 - [ref] counter module: code formated by fmt
Toggle commit list-
59d2e9cd...08bdcde4 - 2 commits from branch
Je reconnais que j'aurais dû automatiquement utiliser fmt AVANT de commiter le code. J'ai décidé de rester cohérent et j'ai pushé du code avec un commit [ref] pour formater le code que j'aurais dû formater avant. @librelois
-
@counter-reverse voici ma review :
commit 1: Le module se nomme toujours skeleton dans le code, il te faut renommer le module dans ce 1er commit en remplaçant toutes les occurrences a Skeleton par un nom approprié. De plus lorsque l'on créer une nouvelle crate dans le projet il faut la déclarée dans le Cargo.toml a la racine du projet.
commit 2 : Tu y a commité un fichier dont les changements n'ont aucun rapport avec le titre du commit, ces changements qui concernent le renommage du module doivent être déplacés dans le 1er commit.
commit 3 : On ne fait jamais de commit spécial fmt, sauf lors d'une mise a jours de fmt ou l'on applique directement, sur dev la dernière version de fmt, tu n'est pas dans ce cas la. Il te faut revenir dans chacun de tes commit appliquer fmt puis amend (tu peut revenir au commit de ton choix avec l'option edit du rebase interactif :)
@librelois J'ai modifié le nom des variables dans le premier commit de la première push request. J'espère que c'était ce que tu voulais.
-
@counter-reverse voici ma review :
commit 1 : Tu a commité des fichiers d'une crate ws2p ?!? Ces fichiers n'ont rien a faire dans ce commit, il faut les supprimer. Aussi bouge l'insertion dans durs-server dans un autre commit comme tu l'avais fait au départ. commit 2 : il faut le squasher avec le commit 1, dans un crer une nouvelle crate il faut la déclarée dans le Carto.toml a la racine, dans le commit ou al crate est créer. commit 3: a sa place dans le 1er commit
Tu ne découpe pas encore de façon pertinente (c'est normal il faut du temps), ici un bon découpage atomique c'et :
- création de ton module vide (donc avec renomage de toute's les occurences a skeleton).
- Injection de topn module dans durs-server
J'aimerais que tu arrive a traiter correctement mes retours avant de passer a la suite :)
@librelois étrange je n'ai jamais eu l'impression d'avoir touché à ws2p.
@librelois la commande git diff semble indiquer que je n'ai pas modifié le fichier ws2p dans aucun des trois commit pushés.
Peut-être que j'ai du mal rebaser.
-
La preuve en image :
Ton 1er commit inclus la modification de 3 fichiers dans ws2p-v1, c'est parce que ta branche n'est pas rebasée sur dev (en tout cas sur le remote, c'est la seule chose que je peut voir je ne peut pas voir ta branche locale, si tu a bien rebaser ta branche locale alors il te faut push force pour que je puisse voir ça).
added 36 commits
-
8c8e69a6...c5a81516 - 34 commits from branch
dev - 5ef44826 - [feat] create counter module
- 4f10ef0e - [feat] main.rs: module counter imported
-
8c8e69a6...c5a81516 - 34 commits from branch
@librelois : la création du module et de son importation sont maintenant séparés dans deux commits différents.
j'en ai profité pour mettre mon module counter à jour.
-
@counter-reverse bien tu progresse mais ce n'est pas encore ça au niveau du nommage du 2ème commit : la portée c'est d'abord nom de la crate et ensuite éventuellemetn le nom du sous module dans la crate voir du fichier si c'est un très gros sous module, mais ici ce n'est pas la cas. De plus
main.rsn'est pas parlant car toutes les crates binaires ont unmain.rs, ici la crate binaire a laquelle tu ajoute ton module c'estdurs-server, al seule existante a ce jours, mais a l'avenir il y aura aussi undurs-desktop(appli de bureau avec fenêtre) voir d'autres crates binaires encore (des nœuds spécialisés).Ensuite dans ton 1er commit tu modifie des fichiers de la crate
durs-server, or ça ne devrait pas être le cas car dans ce commit la tu créer ton module et c'est tout, durs-server n'est pas encore impliqué a ce moment la.
Renommer correctement ton premier commit et déplace les diff de durs-server dans le 2ème et ce sera enfin ok :)
@librelois : Parlant des fichiers de la crate durs-server que je n'aurais pas du modifier dans le premier commit, je pense que j'ai du faire une rebase de la dernière version de la branche dev, modifiant malencontreusement les fichiers.
-
@counter-reverse c'est lr genre de truc a vérifier sur ta GUI git avant de commiter, person je vérifie toujours su mon kraken la liste des fichiers impactés par mon commit avant de commiter. Tu peut faire un rebase interactif avec le mot clés "edit" pour modifier ton 1er commit, il te suffira ensuite d'amend le 2ème.
Tu peut aussi reset soft tes 2 commits puis recommiter tes fichiers bien séparément, il y a plusieurs façon de faire l'important c'est que in finé la séparation soi propre dans l'historique :)
@librelois : après une relecture des commits, je m’aperçois que la modification de bin/durs-server/src/main.rs dans le premier commit est due au formatage automatique de fmt.
Je ne dois surtout pas faire de nouveau commit pour fmt.
Cela pose un problème.
-
@counter-reverse non la modif du fichier
durs-server/Cargo.tomln'a rien a voir avec fmtn tu y a ajouté une ligne qui à sa pla ce dans le commit d'après ;)Pour l'autre fichier oui c'est due a fmt mais je ne vois pas en quoi ça pose problème, qaund tu fait un commit tu n'est pas obligé de commiter tout les fichier sde ton WIP, c'est a ça que sert le stage justement, tu a juste a laisser le fichier dans ton wip et a le stager seulement pour le commit d'après ;)
Edited by Éloïs added 2 commits
@librelois oui c'est vrai pour le fichier durs-server/Cargo.toml.
Je pense avoir fait les modifications adéquates.
-
@counter-reverse ce n'est toujours pas bon, tu a encore commité un fichier de durs-server dans le 1er commit, cela signifie que tu ne maitrise toujours pas le fait de commiter une partie du WIP seulement, c'est pourtant un truc élémentaire qui sert tout le temps.
Comment stage tu tes fichiers, passe tu par kraken ? par vscode ? en cli? Pour ne commiter qu'une partie du wip je te recommande vraiment de passer par une interface graphique (vscode et kraken font ça très bien tout les deux), c'est trop galère en cli.
Cette fois vérifie bien la liste des fichiers dans ton stage avant de commiter, il faut absolument que tu maitrise cette action élémentaire !!
-
@counter-reverse ok dans ce cas il te faut "déstagger" les fichiers a enlever puis amend ton commit puis restagger les fichiers que tu avait enlevé puis créer un nouveau commit puis rebaser continue, ce qui est très compliqué.
Quand tu n'a que deux commit comme ici il est infiniment plus simple de reset soft les 2 commit puis de recommiter les 2 groupes de fichiers séparément.
Rappel pour reset soft les 2 derniers commit :
git reset HEAD^^ --softou
git reset HEAD~2 --soft @librelois le problème avec le premier commit, c'est que fmt a formaté le code automatiquement. Je pense qu'il fera ce changement à chaque formatage.
@librelois je me souviens que le git reset HEAD
2 --hard supprime des commits. Est-ce que la commande git reset HEAD2 --soft est dangereuse?-
@counter-reverse Je sais très bien que c'est fmt qui génère cette modif mais ça ne pose aucun problème, ce n'est pas parce qu'une modif à lieu que tu est obligé de la commiter. Tu peut très bien ne commiter qu'une partie des fichiers modifiées, le reste restant dans ton WIP, c'est ce que je t'ai expliqué plus haut, c'est également expliqué dans la formation git en vidéo que je t'avais envoyé.
De toute evidence, tu ne maîtrise pas encore suffisamment git, il faut que tu creuse encore git car si tu n'est pas capable de faire des commit propre (propre dans le sens ne contenant que les fichiers que tu veut) alors tu ne peut pas contribuer. Courage, tu a déjà bien progresser par rapport au tout début, tu est proche du minimum requis pour pouvoir contribuer :)
L'option --soft permet justement de ne pas perdre les modifications des commiters qui sont reset, les modifs se retrouveront dans ton WIP, c'était également expliqué dans la formation git en vidéo que je t'invite a réviser du coup.
added 2 commits
@librelois ils ne parlaient pas de l'option soft dans la formation. J'ai donc du chercher un peu sur internet. J'y ai aussi trouvé comment commiter seulement certains fichiers en ligne de commande.
J'espère que cette fois, ce sera la bonne. :)
-
J'y ai aussi trouvé comment commiter seulement certains fichiers en ligne de commande.
ils ne parlaient pas de l'option soft dans la formation
Ha ok il me semblais que oui, en tout cas ça t'a forcé a chercher par toi même sur le net, ce que tout bon dev doit savoir faire :)
@counter-reverse Voila l'opération qui consiste a "commiter seulement certains fichiers" vas te servir tout le temps, c'est impossible de faire des petits commit atomiques sans faire ça. Mais c'est une opération chiante a faire en ligne de commande, c'est pourquoi je te recommande vivement de le faire via un logiciel graphqie de ton choix :)
Perso j'utilise git kraken, en cliquant sur ton WIP tout en haut de l'arbre, les fichiers tu WIP apparaissent a droite, en survolant la souris sur chaque fichier tu peut les stager un par un afin de ne stager que les fichiers que tu veut intégrer dans le prochain commit ;)
Par contre tu coup tu a commiter la Cargo.toml racine dans le 2ème commit alors qu'il a sa place dans le 1er : en effet le Cargo.toml a la racine du proejt doit référencer toutes les crates du projet dés leur création. je te laisse modifier ce dernier détail, ça t’entraînera a maîtriser l'opération consistant a commiter seulement certains fichiers :)
added 2 commits
J'ai ajouté Cargo.toml dans le premier commit.
Je n'arrive pas à ignorer le fichier Cargo.lock alors je l'ai commité dans le second commit. J'ai exploré les voies: -git rm Cargo.lock -modifier le gitignore -simplement supprimer le fichier
EDIT: @librelois ah par contre j'ai oublié de formater avec FMT ;) je vais refaire le code.
added 7 commits
-
cba355f2...e0442199 - 5 commits from branch
dev - b804f974 - [feat] create counter module
- d0e98571 - [feat] durs-server: module counter imported
-
cba355f2...e0442199 - 5 commits from branch
@librelois J'ai toujours le fichier Cargo.lock qui m'embête.
-
@counter-reverse ok c'est bien, ça commence a être satisfaisant :)
Pour le fichier Cargo.lock la règle c'est de faire un commit a part pour lui qui se nomme
[deps] update Cargo.lock;)C'est bon tu peut passer a la suite (voir MP sur le forum Duniter)
