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

Refactoring du forum #4621

Closed
wants to merge 14 commits into
base: dev
from

Conversation

Projects
None yet
7 participants
@tleb
Contributor

tleb commented Sep 11, 2017

Q R
Type de modification refactoring
Ticket(s) (issue(s)) concerné(s)

Ca parlait de code pas très beau, donc j'ai fait un tour pour voir si je pouvais aider un peu. Ce ne sont que des refactoring très légers du type, passer une condition devant pour enlever un niveau d'indentation, utiliser str.format(), etc. Je n'ai regardé que la partie forum. Dites moi si je peux faire quoi que ce soit pour simplifier la QA.

QA

  • Un peu tout le code du forum a été touché donc ça risque d'être dur à QA. Je peux faire quoi pour aider ?
Show outdated Hide outdated zds/forum/commons.py Outdated
Show outdated Hide outdated zds/forum/commons.py Outdated
Show outdated Hide outdated zds/forum/commons.py Outdated
Show outdated Hide outdated zds/forum/commons.py Outdated
Show outdated Hide outdated zds/forum/commons.py Outdated
Show outdated Hide outdated zds/forum/commons.py Outdated
Show outdated Hide outdated zds/forum/commons.py Outdated
Show outdated Hide outdated zds/forum/managers.py Outdated
Show outdated Hide outdated zds/forum/models.py Outdated
Show outdated Hide outdated zds/forum/models.py Outdated
Show outdated Hide outdated zds/forum/views.py Outdated
Show outdated Hide outdated zds/forum/views.py Outdated
Show outdated Hide outdated zds/forum/views.py Outdated
@@ -504,12 +560,15 @@ class PostEdit(UpdateView, SinglePostObjectMixin, PostEditMixin):
@method_decorator(transaction.atomic)
def dispatch(self, request, *args, **kwargs):
self.object = self.get_object()
# TODO: couldn't this get refactored?

This comment has been minimized.

@Situphen

Situphen Sep 12, 2017

Contributor

On peut tout mettre dans une seule condition mais ça devient illisible.

@Situphen

Situphen Sep 12, 2017

Contributor

On peut tout mettre dans une seule condition mais ça devient illisible.

This comment has been minimized.

@tleb

tleb Sep 13, 2017

Contributor

en effet :(

@tleb

tleb Sep 13, 2017

Contributor

en effet :(

Show outdated Hide outdated zds/forum/views.py Outdated
Show outdated Hide outdated zds/forum/views.py Outdated
for user in Notification.objects.get_users_for_unread_notification_on(self):
signals.content_read.send(sender=self.topic.__class__, instance=self.topic, user=user)
def solve_attached_alerts(self, user):

This comment has been minimized.

@Situphen

Situphen Sep 12, 2017

Contributor

Est-ce que ça vaut la peine de faire une fonction à part étant donné que c'est utilisé que ici ? De plus, si c'est à part il faut variabiliser le message car il est très spécifique.

@Situphen

Situphen Sep 12, 2017

Contributor

Est-ce que ça vaut la peine de faire une fonction à part étant donné que c'est utilisé que ici ? De plus, si c'est à part il faut variabiliser le message car il est très spécifique.

This comment has been minimized.

@artragis

artragis Sep 13, 2017

Contributor

l'intérêt existe, ne serait-ce que pour rendre le code plus facile à lire.

Maintenant, oui, on pourrait totalement tenter de l'utiliser à d'autres endroits et donc de modifier le message.

@artragis

artragis Sep 13, 2017

Contributor

l'intérêt existe, ne serait-ce que pour rendre le code plus facile à lire.

Maintenant, oui, on pourrait totalement tenter de l'utiliser à d'autres endroits et donc de modifier le message.

Show outdated Hide outdated zds/forum/commons.py Outdated
Show outdated Hide outdated zds/forum/commons.py Outdated
@motet-a

This comment has been minimized.

Show comment
Hide comment
Member

motet-a commented Sep 18, 2017

@zestedesavoir zestedesavoir deleted a comment from coveralls Sep 24, 2017

@tleb

This comment has been minimized.

Show comment
Hide comment
@tleb

tleb Sep 28, 2017

Contributor

Du coup j'ai quoi à faire pour que ça puisse être merge ? J'ai envie de continuer avec les autres sections du site. (:

Contributor

tleb commented Sep 28, 2017

Du coup j'ai quoi à faire pour que ça puisse être merge ? J'ai envie de continuer avec les autres sections du site. (:

@motet-a

This comment has been minimized.

Show comment
Hide comment
@motet-a

motet-a Sep 28, 2017

Member

@tleb Rebase sur dev en corrigeant les conflits et ça devrait être bon vu les reviews qui ont été faites (qui m’ont l’air positives).

Member

motet-a commented Sep 28, 2017

@tleb Rebase sur dev en corrigeant les conflits et ça devrait être bon vu les reviews qui ont été faites (qui m’ont l’air positives).

@motet-a

This comment has been minimized.

Show comment
Hide comment
@motet-a

motet-a Sep 28, 2017

Member

Ah et oublie pas @tleb, on vient de passer à Python 3 à l’instant 🙃

Member

motet-a commented Sep 28, 2017

Ah et oublie pas @tleb, on vient de passer à Python 3 à l’instant 🙃

@tleb

This comment has been minimized.

Show comment
Hide comment
@tleb

tleb Sep 28, 2017

Contributor

J'ai apprécié le moment où j'ai vu des commits 2to3 partout. <3

Je fais le rebase ce soir ou ce week-end.

Contributor

tleb commented Sep 28, 2017

J'ai apprécié le moment où j'ai vu des commits 2to3 partout. <3

Je fais le rebase ce soir ou ce week-end.

@tleb tleb referenced this pull request Oct 8, 2017

Closed

Forum refactoring #4707

@motet-a

This comment has been minimized.

Show comment
Hide comment
@motet-a

motet-a Oct 8, 2017

Member

Super ! Il va juste falloir que tu nettoies les choses mal mergées.

Member

motet-a commented Oct 8, 2017

Super ! Il va juste falloir que tu nettoies les choses mal mergées.

@motet-a motet-a changed the title from Petit refactoring forum to Refactoring du forum Oct 8, 2017

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 8, 2017

Coverage Status

Coverage decreased (-33.8%) to 55.885% when pulling cb7fd10 on tleb:changes into fd39c47 on zestedesavoir:dev.

coveralls commented Oct 8, 2017

Coverage Status

Coverage decreased (-33.8%) to 55.885% when pulling cb7fd10 on tleb:changes into fd39c47 on zestedesavoir:dev.

@zestedesavoir zestedesavoir deleted a comment from coveralls Oct 8, 2017

@tleb tleb referenced this pull request Oct 8, 2017

Closed

Member refactoring #4709

@tleb

This comment has been minimized.

Show comment
Hide comment
@tleb

tleb Oct 8, 2017

Contributor

C'est bon, les conflits ont été résolus. 🙃

Contributor

tleb commented Oct 8, 2017

C'est bon, les conflits ont été résolus. 🙃

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 8, 2017

Coverage Status

Coverage decreased (-33.8%) to 55.885% when pulling 2f1d8f9 on tleb:changes into 99fe388 on zestedesavoir:dev.

coveralls commented Oct 8, 2017

Coverage Status

Coverage decreased (-33.8%) to 55.885% when pulling 2f1d8f9 on tleb:changes into 99fe388 on zestedesavoir:dev.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Oct 11, 2017

Coverage Status

Coverage decreased (-33.8%) to 55.888% when pulling 471c37b on tleb:changes into 3f0531a on zestedesavoir:dev.

coveralls commented Oct 11, 2017

Coverage Status

Coverage decreased (-33.8%) to 55.888% when pulling 471c37b on tleb:changes into 3f0531a on zestedesavoir:dev.

@pierre-24

This comment has been minimized.

Show comment
Hide comment
@pierre-24

pierre-24 Aug 3, 2018

Member

Plus de source :(

Member

pierre-24 commented Aug 3, 2018

Plus de source :(

@pierre-24 pierre-24 closed this Aug 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment