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

Verification backside formulaire ajout karma (#3400) #3436

Closed
wants to merge 5 commits into from
Closed

Verification backside formulaire ajout karma (#3400) #3436

wants to merge 5 commits into from

Conversation

MatthieuLepers
Copy link

Q R
Correction de bugs ? [oui]
Nouvelle Fonctionnalité ? [oui]
Tickets (issues) concernés #3400

QA:
Côté front on a le symbole « * » rouge qui indique que le champs Commentaire est requis mais il est possible d'ajouter une remarque sans commentaire.

2 options :

C'est côté front qu'on a un mauvais affichage;
C'est côté back et on check mal si le formulaire est valide (je penche pour ça).

note
La vérification comprend :

  • Commentaire vide
  • Intervalle de la valeur karma
    Dans le cas où ces informations sont incorrectes, la validation du formulaire n'ajoute ni le commentaire ni la valeur karma au profil du membre.

@MatthieuLepers MatthieuLepers changed the title Verification backside formulaire ajout karma Verification backside formulaire ajout karma (#3400) Mar 11, 2016
@artragis
Copy link
Member

Oups, tu as des problèmes de pep8. Pep8 c'est la norme de style de codage en python, elle fait même partie des choses officielles du langage.

Pour éviter que le code ne perde en lisibilité, nous utilisons un outil qui vérifie que tous les commits sont en phase avec cette norme, c'est pour ça que travis fail. Il faut donc que tu fasses deux/trois modifs dans ton code.

Je te conseille aussi d'utiliser le githook qu'on conseille dans la doc (sauf si tu utilises windows car là ça devient plus délicat)

@MatthieuLepers
Copy link
Author

Bah sinon si t'a deux secondes regarde mes ajouts et envoi moi la ligne qui va pas, je la corrigerai de suite ;)

@artragis
Copy link
Member

Il suffit de regarder le résultat du build de travis, il te les donne.

Le 11/03/2016 18:42, Matthieu Lepers a écrit :

Bah sinon si t'a deux secondes regarde mes ajouts et envoi moi la
ligne qui va pas, je la corrigerai de suite ;)


Reply to this email directly or view it on GitHub
#3436 (comment).

@MatthieuLepers
Copy link
Author

Cette fois l'indentation est bonne, mais Travis à refusé, je vois pas bien d'où sa provient cette fois ci.
Je squasherai tous sa après

@artragis
Copy link
Member

T'inquiète c'est une erreur interne de travis, tu peux squasher, tout est bon de ton côté.

Lepers Matthieu and others added 2 commits March 11, 2016 20:24
Modif. indentation pour respecter norme flake8

Travis devrait passé avec cette modif.
@MatthieuLepers
Copy link
Author

Voilà j'ai squash, ne tien pas compte du Merge branch

profile.karma += note.value
profile.save()
except (ValueError):
""" Nothing """
Copy link
Member

Choose a reason for hiding this comment

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

Peux-tu mettre un peu de logging, c'est pas encore la norme sur zds, mais les dernières release ont montré que ça peut être plus que nécessaire.

De plus il faut gérer ça en ajoutant un message d'erreur dans les messages.

Copy link
Author

Choose a reason for hiding this comment

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

Il me faudrait un exemple si tu as sa sous la main

Copy link
Member

Choose a reason for hiding this comment

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

@MatthieuLepers
Copy link
Author

Bon je pense pas allé jusqu'au bout de cette issue, je galère trop avec les tests. Je vous laisse cette PR et les autres, à vous de voir si vous récupéré ou non.

@SpaceFox
Copy link
Contributor

Reprise par #3449

@SpaceFox SpaceFox closed this Mar 19, 2016
gustavi pushed a commit that referenced this pull request Mar 21, 2016
Reprise de #3436 : vérification du karma
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.

None yet

3 participants