Ameliorations de code conseillées par landscape #2712
Conversation
| # Index | ||
| url(r'^$', ListGallery.as_view(), name='gallery-list'), | ||
| ) | ||
| # Add and edit a gallery |
There was a problem hiding this comment.
Ne peut-on pas plutôt modifier la règle ? Pour moi, ici l'ancienne version était bien plus lisible que la nouvelle, et l'important, c'est la lisibilité et pas les règles.
|
|
|
|
|
|
1 similar comment
|
|
|
|
|
Y'a encore des trucs à faire, par ici ? (à priori, j'ai rien vu de farfelu dans le code). |
|
Oui, j'attends le merge de la PR de @GerardPaligot sur le module des forums pour repasser sur ce dernier si nécessaire. |
|
Vu :) |
|
Voila, edit des forums fait en me rebasant sur dev... Cependant c'est du push sauvage, j'ai pas eu le temps de lancer les tests sur ma machine ce soir... (du coup "on verra Travis" dans un premier temps) |
|
|
|
Hum, plein d'erreurs, toutes sur les fofos... je regarde ca... |
|
Bon voila normalement tout passe. Ca m'aurait fait corrige des soucis dans le code qui passait inaperçu a cause d' |
|
|
| @@ -760,7 +760,7 @@ def test_filter_member_ip(self): | |||
| # Check that the filter can't be access from normal user | |||
| result = self.client.post( | |||
| reverse('zds.member.views.member_from_ip', | |||
| kwargs={'ip': tester.last_ip_address}), | |||
| kwargs={'ipadress': tester.last_ip_address}), | |||
|
Bon, petit push pour avoir un coverage/landscape/travis AVANT le rebase de la PR d' @artragis <3 |
|
|
|
ET PAF le rebase de la PR d'artragis qui c'est fait tout seul (sauf pour |
|
|
|
|
| @@ -102,7 +102,7 @@ def items(self, obj): | |||
| if "forum" not in obj and "tag" not in obj: | |||
| topics = Topic.objects.filter(forum__group__isnull=True)\ | |||
| .order_by('-pubdate')[:settings.ZDS_APP['forum']['posts_per_page']] | |||
| except: | |||
| except (Post.DoesNotExist, ValueError): | |||
There was a problem hiding this comment.
C'est pas plutôt Topic.DoesNotExist ici ?
There was a problem hiding this comment.
Si. C'est ce qui se passe quand on laisse un boulet copier/coller !
|
|
|
D'autres choses à dire ? |
Perso je n'ai plus aucune remarque à faire. |
| @@ -592,11 +592,11 @@ def followed_topics(request): | |||
| Displays the followed topics fo the current user, with `settings.ZDS_APP['forum']['followed_topics_per_page']` | |||
| topics per page. | |||
| """ | |||
| followed_topics = request.user.profile.get_followed_topics() | |||
| topics_followed = request.user.profile.get_followed_topics() | |||
There was a problem hiding this comment.
Pourquoi ? Je trouvais ça mieux followed_topics !
There was a problem hiding this comment.
parce que c'est le nom de la fonction
|
A part mes quelques commentaires, ça m'a l'air tout bon pour le merge ! |
|
Voili voilou ! |
|
|
|
Vu que @SpaceFox @GerardPaligot @artragis @firm1 et moi on a commenté, que Travis est OK et qu'on ne va pas QA tout le site (car ça touche à tout), je merge ! |
Ameliorations de code conseillées par landscape
|
@GerardPaligot le coverage ne compte pas ici, je ne touche pas au test. C'est le score landscape qui est cense bouger :) |
|
Tu as été rapide. J'ai supprimé mon message direct en comprenant que c'était sur landscape que tu avais bossé et pas coveralls. :) |
Ce ticket propose quelques améliorations de code conseillées par landscape. Il y a encore du boulot mais c'est un début.
Grosso modo :
list) ont été remplace pour éviter les confusionsJ'ai volontairement ignoré les modules articles, tutos et mp (car les deux premiers vont beaucoup changer avec la ZEP-12 et le module tuto peut-etre aussi avec l'API en dev dessus)