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

Refactor zds/tutorialv2/models/__init__.py #4762

Merged
merged 1 commit into from
Nov 10, 2017

Conversation

motet-a
Copy link
Contributor

@motet-a motet-a commented Oct 30, 2017

Renomme certaines variable que je trouvais bizarrement nommées.

@motet-a motet-a added C-Back Concerne le back-end Django S-Refactorisation Améliore le code existant sans forcément ajouter de nouvelle fonctionnalité labels Oct 30, 2017
@motet-a motet-a force-pushed the refactor-zds.tutorialv2.models branch from 9078922 to 07d7a89 Compare October 30, 2017 10:07
Copy link
Member

@gllmc gllmc left a comment

Choose a reason for hiding this comment

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

Merci pour ce refactoring, les noms des variables sont en effet bien plus cohérents !

('FORUM', _('Forum')),
('CONTENT', _('Contenu')),
) + TYPE_CHOICES
] + TYPE_CHOICES
Copy link
Member

Choose a reason for hiding this comment

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

Pourquoi utiliser une liste plutôt qu'un tuple ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parce qu’avant c’était un tuple litéral dans les réglages et que j’ai changé ça en liste et qu’on ne peut pas concaténer facilement un tuple avec une liste avec l’opérateur +.

Puis j’imagine qu’en termes de performances, la différence entre un tuple et une liste est négligeable dans ce cas.

content['name']
for content in CONTENT_TYPES
if content['beta']
]
Copy link
Member

Choose a reason for hiding this comment

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

J'avoue que je suis moyennement convaincu par le passage des compréhensions de liste sur plusieurs lignes. Je trouve que c'est largement lisible sur une ligne avec la limite des 120 caractères.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh pas de souci, je remet comme c’était avant 🙃

(J’ai passé de longues années bloqué à la colonne 79 et j’ai un peu du mal à m’y défaire)

@motet-a motet-a force-pushed the refactor-zds.tutorialv2.models branch 2 times, most recently from e417996 to 7a84d98 Compare October 30, 2017 19:03
@zestedesavoir zestedesavoir deleted a comment from coveralls Oct 30, 2017
@zestedesavoir zestedesavoir deleted a comment from coveralls Oct 30, 2017
@motet-a motet-a force-pushed the refactor-zds.tutorialv2.models branch from 7a84d98 to 7f51f51 Compare October 30, 2017 19:10
@zestedesavoir zestedesavoir deleted a comment from coveralls Oct 31, 2017
@zestedesavoir zestedesavoir deleted a comment from coveralls Oct 31, 2017
@zestedesavoir zestedesavoir deleted a comment from coveralls Oct 31, 2017
Copy link
Member

@gllmc gllmc left a comment

Choose a reason for hiding this comment

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

N'oublie pas de générer les migrations avec python manage.py makemigrations, elles sont manquantes. ;)

@artragis : tu as mis un 👎 plus haut, pourquoi ?

@motet-a motet-a force-pushed the refactor-zds.tutorialv2.models branch from 7f51f51 to bbe38d0 Compare November 4, 2017 10:35
@motet-a
Copy link
Contributor Author

motet-a commented Nov 4, 2017

@gcodeur C’est fait, mais là je ne comprends pas du tout pourquoi il y aurait besoin de migrations. Je n’ai pas touché au modèle, non ?

@motet-a
Copy link
Contributor Author

motet-a commented Nov 4, 2017

Ah, je viens de comprendre. C’est une histoire de majuscules.

@motet-a motet-a force-pushed the refactor-zds.tutorialv2.models branch from bbe38d0 to 70c0ae6 Compare November 4, 2017 10:47
@zestedesavoir zestedesavoir deleted a comment from coveralls Nov 4, 2017
@motet-a motet-a force-pushed the refactor-zds.tutorialv2.models branch from 70c0ae6 to 323e658 Compare November 4, 2017 11:03
@zestedesavoir zestedesavoir deleted a comment from coveralls Nov 4, 2017
@zestedesavoir zestedesavoir deleted a comment from coveralls Nov 4, 2017
Renomme certaines variable que je trouvais bizarrement nommées.
@motet-a motet-a force-pushed the refactor-zds.tutorialv2.models branch from 323e658 to 099c5d9 Compare November 10, 2017 18:33
@coveralls
Copy link

coveralls commented Nov 10, 2017

Coverage Status

Coverage remained the same at 89.801% when pulling 099c5d9 on motet-a:refactor-zds.tutorialv2.models into 85e56e2 on zestedesavoir:dev.

@zestedesavoir zestedesavoir deleted a comment from coveralls Nov 10, 2017
@motet-a motet-a merged commit 78d1f87 into zestedesavoir:dev Nov 10, 2017
@motet-a motet-a deleted the refactor-zds.tutorialv2.models branch November 10, 2017 18:53
@motet-a motet-a added this to the Version de développement milestone Nov 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Back Concerne le back-end Django S-Refactorisation Améliore le code existant sans forcément ajouter de nouvelle fonctionnalité
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants