Merge lp://staging/~bloodearnest/django-preflight/gargoyle2 into lp://staging/django-preflight

Proposed by Simon Davy
Status: Merged
Approved by: Michael Foord
Approved revision: 26
Merged at revision: 25
Proposed branch: lp://staging/~bloodearnest/django-preflight/gargoyle2
Merge into: lp://staging/django-preflight
Diff against target: 490 lines (+248/-22)
10 files modified
.bzrignore (+1/-0)
doc/conf.py (+1/-1)
doc/install.rst (+3/-3)
preflight/models.py (+36/-0)
preflight/templates/preflight/overview.html (+68/-7)
preflight/tests.py (+111/-1)
preflight/views.py (+5/-0)
preflight_example_project/run.py (+2/-2)
preflight_example_project/settings.py (+7/-2)
setup.py (+14/-6)
To merge this branch: bzr merge lp://staging/~bloodearnest/django-preflight/gargoyle2
Reviewer Review Type Date Requested Status
Michael Foord (community) Approve
Review via email: mp+166845@code.staging.launchpad.net

Commit message

Add support for gargoyle flags in preflight

Description of the change

Add support for gargoyle flags in preflight

To post a comment you must log in.
Revision history for this message
Simon Davy (bloodearnest) wrote :
Revision history for this message
Michael Foord (mfoord) wrote :

The code should not depend on settings.USE_GARGOYLE.

Other than that all looks good.

review: Needs Fixing
Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

- swap l. 72 and 73 around
- l 109, 120, 143, 155: fix whitespace before :
- l.280: why not add in l.270?
- l.398: what's the need of renaming the example_project folder?
- l.428: since you renamed the db, why add the old name to bzrignore? (l. 9)

Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

Can you also include a screenshot of how the new template looks like with the flags?

Revision history for this message
Simon Davy (bloodearnest) wrote :

> The code should not depend on settings.USE_GARGOYLE.
>
> Other than that all looks good.

Done

Revision history for this message
Simon Davy (bloodearnest) wrote :

> - swap l. 72 and 73 around
> - l 109, 120, 143, 155: fix whitespace before :
> - l.280: why not add in l.270?

Done the above

> - l.398: what's the need of renaming the example_project folder?

This is unfortunately necessary because the nexus package (a dep of gargoyle) is higher in sys.pth when run with "python setup.py test", and it erroneously includes its example_project directory, meaning that the test_suite in setup.py ("example_project.run.tests") was finding the wrong example_project module and failing.

Pull request to exclude example_project upstream: https://github.com/disqus/nexus/pull/5

> - l.428: since you renamed the db, why add the old name to bzrignore? (l. 9)

Done - a left over from earlier

26. By Simon Davy

addressing review comments

Revision history for this message
Simon Davy (bloodearnest) wrote :
Revision history for this message
Michael Foord (mfoord) wrote :

Good work. Thanks.

review: Approve

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