Ajout de commentaires et remarques sur le module des forums #1945
Conversation
| """ | ||
| :return: the first post of a topic, written by topic's author. | ||
| """ | ||
| # TODO: Why force the relation with author? Why "the first post" is not sufficient? |
There was a problem hiding this comment.
C'est un peu jeu de piste, mais la réponse se trouve quelques lignes plus haut dans get_last_answer() : à un moment, il y a une comparaison qui appelle cette fonction. Je pense que pour que cette comparaison soit juste, il faut ce "related". Ceci dit, si on remplaçait, à la ligne 236,
if last_post == self.first_post():par
if last_post.pk == self.first_post().pk:ça serait surement plus nécéssaire.
SpaceFox
force-pushed
the
code_comments
branch
from
5aba000
to
4c9c6f0
Compare
|
|
||
| """A forum, containing topics.""" | ||
| """ | ||
| A Forum, containing Topics. It can be public or restricted to some groups. |
There was a problem hiding this comment.
redondant avec la ligne 92. l'ancien commentaire était bon.
There was a problem hiding this comment.
Redondant dans le code source peut-être, mais les docstring (et seulement les docstrings) vont permettre de générer une doc "externe" du code. Donc pour moi cette précision doit y figurer.
There was a problem hiding this comment.
Dans ce seul cas (parce qu'une documentation sera générée), je suis d'accord avec @SpaceFox.
Eskimon
added
C-Back
ca peut etre ? http://sphinx-doc.org/ext/autodoc.html |
| class Meta: | ||
| verbose_name = 'Catégorie' | ||
| verbose_name_plural = 'Catégories' | ||
|
|
||
| title = models.CharField('Titre', max_length=80) | ||
| position = models.IntegerField('Position', null=True, blank=True) | ||
| # Some category slugs are forbidden due to path collisions: Category path is `/forums/<slug>` but some actions on | ||
| # forums have path like `/forums/<action_name>`. Forbidden slugs are all top-level path in forum's `url.py` module. | ||
| # As Categories can only be managed by superadmin, this is purely declarative and there is no control on slug. |
There was a problem hiding this comment.
Ne vaut-il pas mieux intégrer toutes ces explications dans la propriété help_text du champ (qui en l'état ne donne aucune information) ?
Au passage, le contenu de ce commentaire est aberrant.
There was a problem hiding this comment.
Sisi, le help_text donne les informations mais une partie des explications est bouffée par l'interface Github.
Qu'est-ce que tu entends par "aberrant" ?
There was a problem hiding this comment.
Qu'est-ce que tu entends par "aberrant" ?
Avoir une URL partagée pour deux choses différentes.
There was a problem hiding this comment.
Ah, c'est pas le commentaire qui est aberrant mais le fonctionnement. Mais là oui, je suis d'accord, ça faisait partie des problèmes d'URLs que je mentionnais pendant le zest meeting tout à l'heure.
|
Certes, mais à quelle échéance ? Rester avec des tas de conneries dans le Le 14 janvier 2015 23:15, Gerard Paligot notifications@github.com a écrit
|
|
Impossible de te dire quand sortira l'API. N'importe quoi peut survenir pendant le développement de ses ZEPs (je compte rédiger aujourd'hui la prochaine sur les MPs pour enchainer avec après celle sur les membres). Tout ce que je peux t'assurer c'est que la plupart des commentaires qui vont figurer dans les fichiers Donc, je ne dis pas que ça serait mal de documenter les modules. Cela pourrait même aider pour l'API pour les morceaux de code trop chaud à comprendre sans commentaire mais il faut savoir qu'ils seront probablement dégagés après le passage de l'API. |
|
J'ai bien compris la réponse à la question 2, elle m'a été donnée au moins 3 fois. Par contre j'aimerais bien une réponse claire et précise aux questions 1 et 3 :
Je veux dire, une réponse ici. Pas un listing des problèmes rencontrés dans les TODO ou des remarques sur un point de détail. C'est d'un commentaire général dont j'ai besoin, parce que ce genre de travail prends un temps non négligeable et que j'ai besoin de savoir comment le calibrer. Et je vous avoue que pour l'instant le manque de réponse à ce sujet et les commentaires de @GerardPaligot m'ont passablement démotivés. Entre autres parce que ça me donne clairement l'impression que soit tout le monde se fout du fait que notre code est en partie incompréhensible (dans les sens où on a des méthodes que plus personne ne comprends !) et que globalement les contributeurs préfèrent le code crade à passer 2 minutes à mettre le minimum de commentaires. |
|
Tout d'abord, je dirais que je suis très satisfait avec le niveau de commentaires que tu as effectué et la qualité de ceux-ci. Ce genre de PR doit pour moi être poursuivie, à tout pris : on sais pas quand une API "forum" arrivera et faire rustine d'ici là, c'est ne faire qu'empirer les choses. Et mieux vaut tard que jamais. (si j'ai laissé croire que je m'en foutais, my bad, mais c'est absolument pas le cas). Ceci dit, il y a pour moi deux problèmes ici :
|
|
Je crois que tu as confondu @Ge0 et @GerardPaligot :D Cela dit, certains de mes commentaires mériteraient en effet d'être remplacés par un "TODO : renommer cette méthode en |
Très sincèrement, je suis désolé parce que ce n'était pas du tout l'effet voulu. J'aimerais vraiment que tu relise cette partie de mon message précédent :
En gros, s'il y a des passages incompréhensibles, je suis pour les commentaires parce que quelqu'un (moi peut-être mais pas forcément) devra comprendre le code pour le refactorer et si la personne ne le comprend pas, elle pourrait mal faire.
Hormis certains commentaires où il faudrait plus expliciter le nom de la méthode (ou le code), je suis plutôt pour. Tu places les commentaires uniquement sur les classes et les méthodes et uniquement quand c'est nécessaire partout ailleurs (passage dans le code complexe ou informations complémentaires sur un morceau de code précis).
La limit entre un commentaire fonctionnel et d'implémentation est assez mince je trouve. Cependant, si aucun commentaire se raccroche à des éléments dans le code et si le commentaire est compréhensible (parfois, j'ai juste pas compris), je trouve le niveau de détail plutôt correct. Voilà, faut que j'arrête de dire toujours que le négatif. :) |
|
Petit UP ! Est-ce qu'il y a d'autres commentaires ? |
|
Je vais d'abord rebase et voir ensuite. |
SpaceFox
force-pushed
the
code_comments
branch
3 times, most recently
from
18b2610
to
73fe102
Compare
|
Bon j'ai corrigé 2/3 trucs. Pour moi on peut déjà merger cette partie. |
|
Bon, moi je dis "vas-y". Ou on ne l'aura jamais. |
|
Erreur flake8 ici. |
|
C'est corrigé. |
|
Ouais, mais faut rebase maintenant. :-° |
|
C'est rebase. Si Travis est content, on peut merger. |
|
Travis est content. Merge qui veut. |
|
ping @Eskimon 2015-04-03 8:51 GMT+02:00 SpaceFox notifications@github.com:
|
Ajout de commentaires et remarques sur le module des forums
|
pong |
|
J'ai mis ca en 1.8, @SpaceFox c'est cohérent ? |
|
Oui. |
Le but de cette PR est de rajouter des commentaires, qui manquent énormément dans le code.
Lors de mes pérégrinations, j'ai aussi trouvé un certain nombre de bizarreries, ici signalées par des TODO, parce que ce n'est pas le but de la PR de les corriger et que certaines ne méritent probablement même pas une issue.
Je propose qu'on merge déjà ça, et qu'on fasse plusieurs petites PR.