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

Clarifie l'estimation du temps de lecture #5991

Closed
wants to merge 4 commits into from
Closed

Clarifie l'estimation du temps de lecture #5991

wants to merge 4 commits into from

Conversation

Benallegue-Mehdi
Copy link
Contributor

@Benallegue-Mehdi Benallegue-Mehdi commented Nov 14, 2020

Tente de résoudre l'issue #5780.

Q/A

  1. Prendre l'archive du tutoriel Arduino
  2. Le publier
  3. Regarder l'estimation du temps de lecture
  4. Constatez que c'est 1 jour 6h

Comportement attendu

L'estimation est de 30h.

Comportement obtenu

L'estimation est de moins d'une minute.

Le template fonctionne : l'erreur est dûe à un comportement bizarre qui affecte probablement les dernières lignes de tags_authors.part.html. Il faudra donc y jeter un oeil, sachant que je n'ai pas les compétences nécessaires pour le faire par moi-même.

Merci !

@Arnaud-D Arnaud-D added this to En attente de QA in Suivi des PR Nov 22, 2020
@Arnaud-D Arnaud-D moved this from En attente de QA to En développement in Suivi des PR Nov 22, 2020
@AmauryCarrade AmauryCarrade marked this pull request as draft November 22, 2020 20:50
@AmauryCarrade
Copy link
Member

AmauryCarrade commented Nov 23, 2020

Le code de tox fonctionne en l'état, on l'a testé, par contre il ne fonctionne pas sur des tutoriels à cause d'un bug détecté à l'occasion et décrit dans #5995. Pour la QA, il faut donc absolument tester sur un article.

Aussi, @ToxicScorpius, il faudra mettre à jour les tests unitaires de minute_to_duration, qui, ne passent plus vu que tu as modifié le comportement du filtre :) . Il suffit de reprendre les tests existants en mettant à jour ce qu'on est censé obtenir, ou corriger le code pour que les tests passent, fonction de ce qu'on veut.

En fait, les tests s'attendent à la présence d'une espace insécable entre les nombres et leurs mot (p.ex. 2 heures), et la nouvelle implémentation n'en met pas. Je pense qu'il serait pertinent de garder ces espaces insécables pour éviter des coupures disgracieuses.

Il serait également pertinent d'ajouter d'autres tests sur le même modèle, afin de tester qu'on a bien uniquement des heures et non des jours pour de grandes valeurs — ce qu'a modifié cette PR, quoi.

@Benallegue-Mehdi
Copy link
Contributor Author

J'ai modifié les tests, découvrant un bug au passage, promptement corrigé. Les espaces insécables ont été conservés. Je pense que la PR est prête pour un commit et qu'il serait plus pertinent de corriger l'erreur derrière de manière séparée vu que ce bug affecte aussi la prod.

@Benallegue-Mehdi Benallegue-Mehdi marked this pull request as ready for review November 23, 2020 02:23
@Benallegue-Mehdi Benallegue-Mehdi changed the title Clarifie l'estimation du temps de lecture (non fonctionel) Clarifie l'estimation du temps de lecture Nov 23, 2020
Suivi des PR automation moved this from En développement to Cimetière Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Suivi des PR
  
Cimetière
Development

Successfully merging this pull request may close these issues.

None yet

2 participants