Merge lp://staging/~nataliabidart/ubuntu-webcatalog/law-and-order into lp://staging/ubuntu-webcatalog

Proposed by Natalia Bidart
Status: Superseded
Proposed branch: lp://staging/~nataliabidart/ubuntu-webcatalog/law-and-order
Merge into: lp://staging/ubuntu-webcatalog
Diff against target: 20 lines (+2/-1)
1 file modified
src/webcatalog/models/applications.py (+2/-1)
To merge this branch: bzr merge lp://staging/~nataliabidart/ubuntu-webcatalog/law-and-order
Reviewer Review Type Date Requested Status
Michael Nelson (community) Approve
Review via email: mp+108204@code.staging.launchpad.net

Commit message

- Fixed failing tests due to database ordering issues.
- Fixed all the new PEP-8 issues raised by the new version of the pep8 checker.

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :

Thanks Natalia!

With the application media ordering, it might be safer (for the future) to set the default ordering on the ApplicationMedia class, rather than on that one call-site (_get_media()).

Anyway, +1 either way (once the conflict is gone).

review: Approve
Revision history for this message
Michael Nelson (michael.nelson) wrote :

Actually, I just noticed you've switch various view orderings to use the settings.DEFAULT_SORT_FIELD - why is that?

Wouldn't in make more sense to set the Application classes default sort ordering to whatever that setting is, and leave the various views with their customised sort order? I'm probably missing some info that would clear this up, but just want to be sure.

-Michael

review: Needs Information
Revision history for this message
Michael Nelson (michael.nelson) wrote :

OK, I think I've got it - the bug here is that the various Application views use inconsistent sorting. In which case, I reckon:

s/default_sort_field/application_views_sort_by/

(as default_sort_field could be for any model, and 'default' confused me with the django default model ordering, but if you don't think that's an issue, fine to leave it as is.).

review: Approve
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Noodles,

I'm sorry you had to try to understand this mess... the problem was that I worked on 2 different things using the same branch (since the former was a per-requisite of the latter), and when I pushed the second branch, I forgot to set explicitly the (new) location.

So, long story short, I've cleaned up this branch, and will be pushing then ASAP.

132. By Łukasz Czyżykowski

[r=michael.nelson],[bug=1006442] Cope with doubled applications in the database.

133. By Natalia Bidart

- Fixed tests failures caused by DB ordering issues (LP: #1009587).

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches