n7consulting / Incipio

ERP / CRM for Junior-Entreprises.
http://jeyser-crm.n7consulting.fr
GNU Affero General Public License v3.0
42 stars 17 forks source link

Various fixes - "Devis" Generation is now available #337

Closed KiDayz closed 3 years ago

KiDayz commented 3 years ago

Medium fixes

Minor fixes

Stoakes commented 3 years ago

Merci beaucoup pour cette contribution.

Il y a beaucoup de choses bien donc elle sera probablement mergée, une fois la CI verte & les commentaires corrigés (git rebase -i c'est la vie).

Par contre, je ne suis pas fan des 2 commits

Voilà ce que j'ai en tête à leur sujet: (cette liste est là pour ouvrir la discussion, pas pour chercher à détruire ces commits)

A part ça, GG

KiDayz commented 3 years ago

Hello,

Merci pour tes corrections, je m'occuppe de regarder ça. Par contre je n'ai jamais utilisé git rebase, peux-tu m'expliquer rapidement pourquoi je devrais l'utiliser ? :)

C'est pour éviter de faire un commit sur ma branche avec les changes puis de refaire une PR ?


PVRI / PVRF

Merci pour tes commentaires et ton ouverture d'esprit, voilà ce que je peux apporter à la discussion :

Stoakes commented 3 years ago

PVR Ok, faisons comme ça.

git rebase -i est une commande git qui permet de re-écrire ton historique de commit. C'est particulièrement utile pour maintenir une liste de commit cohérente (et éviter les commits à la Address review comment ou encore Lint code).

Au lieu d'ajouter de nouveaux commits avec les modifications liées à la review, git rebase -i te permet de modifier les commits précédents, puis de les repousser (il faudra faire un git push --force pour forcer la ré-ecriture de l'historique).

Cet article explique plus en détails: https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History (la partie "The Nuclear Option: filter-branch" et ce qui suit est moins utile).

Est ce que c'est bon ou tu souhaites plus de détails sur comment l'utiliser dans le cas de cette PR ?

KiDayz commented 3 years ago

Ok super merci pour l'explication, j'ai mis un peu de temps à tout comprendre et tout faire pour la première fois, mais ça devrait être bon. (le lien et tes explications étaient super utiles merci !)

KiDayz commented 3 years ago

@Stoakes Bon les checks sont passés, j'ai rebase pour n'avoir plus que les 6 commits existants, j'espère ne pas avoir oublié l'une de tes remarques mais je ne crois pas ^^ on y arrive doucement

KiDayz commented 3 years ago

j'ai ajouté les modifs que j'ai fais ce soir sur les graphs. J'espère que ça ne perturbe pas le process de review de la PR ?

Stoakes commented 3 years ago

On y est presque. Il ne reste plus que le && à la place du and et ca sera mergé.