Skip to content
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

Casser counter click étape 1 : introduire des fragments #921

Open
wants to merge 2 commits into
base: taiste
Choose a base branch
from

Conversation

klmp200
Copy link
Contributor

@klmp200 klmp200 commented Nov 14, 2024

CounterClick est gros, CounterClick est dangereux, CounterClick est important.

Ce serait judicieux de le splitter en plusieurs petits morceaux pour le rendre plus digeste, plus extensible, plus rapide et plus testable.

La première étape ici c'est de sortir la gestion du formulaire des cartes étudiantes et de la déléguer à une autre vue.

Dépend de #920

Copy link
Contributor

@Hyask Hyask left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sur le principe, c'est très cool, mais je sais pas trop pourquoi tu veux que je review ça. Déjà la partie NFC, j'étais plus là quand tu l'as ajouté, et je l'ai jamais vu fonctionner XD

Sur le côté architectural, ça semble OK, mais pareil, webpack j'ai jamais touché, typescript non plus, donc c'est quand même limité :-)

@klmp200
Copy link
Contributor Author

klmp200 commented Nov 14, 2024

Sur le principe, c'est très cool, mais je sais pas trop pourquoi tu veux que je review ça. Déjà la partie NFC, j'étais plus là quand tu l'as ajouté, et je l'ai jamais vu fonctionner XD

Sur le côté architectural, ça semble OK, mais pareil, webpack j'ai jamais touché, typescript non plus, donc c'est quand même limité :-)

C'est que le dernier commit qui est important ici :)
Comme j'ai dit dans la description ça dépend d'une autre pull request :)
Merci pour la review au moins au niveau de l'architecture, ça fait plaisir :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants