Merge lp://staging/~canonical-isd-hackers/django-preflight/gargoyle into lp://staging/django-preflight

Proposed by Michael Foord
Status: Needs review
Proposed branch: lp://staging/~canonical-isd-hackers/django-preflight/gargoyle
Merge into: lp://staging/django-preflight
Diff against target: 304 lines (+118/-24)
8 files modified
.bzrignore (+2/-0)
doc/install.rst (+6/-6)
example_project/settings.py (+1/-0)
preflight/models.py (+15/-0)
preflight/templates/preflight/overview.html (+50/-7)
preflight/tests.py (+31/-1)
preflight/views.py (+3/-0)
tox.ini (+10/-10)
To merge this branch: bzr merge lp://staging/~canonical-isd-hackers/django-preflight/gargoyle
Reviewer Review Type Date Requested Status
Canonical ISD hackers Pending
Review via email: mp+106374@code.staging.launchpad.net

Commit message

Add gargoyle support (display switches on the preflight page).

To post a comment you must log in.
Revision history for this message
ISD Branch Mangler (isd-branches-mangler) wrote :

No approved revision specified.

Revision history for this message
ISD Branch Mangler (isd-branches-mangler) wrote :

No approved revision specified.

Revision history for this message
ISD Branch Mangler (isd-branches-mangler) wrote :

No approved revision specified.

Revision history for this message
ISD Branch Mangler (isd-branches-mangler) wrote :

No approved revision specified.

Revision history for this message
ISD Branch Mangler (isd-branches-mangler) wrote :

No approved revision specified.

Revision history for this message
ISD Branch Mangler (isd-branches-mangler) wrote :

No approved revision specified.

Revision history for this message
ISD Branch Mangler (isd-branches-mangler) wrote :

No approved revision specified.

Revision history for this message
ISD Branch Mangler (isd-branches-mangler) wrote :

No approved revision specified.

Revision history for this message
Anthony Lenton (elachuni) wrote :

Hi Michael!

This looks great, my only concern is, it seems like this assumes you have Gargoyle installed. What happens if you don't have gargoyle installed or available?

Also, the last few lines seem to be asking for "django >= 1.4, < 1.1" in a couple of places, which I think will never match?

Thanks!

achuni.

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

Some ideas for improvements

- rename gather_gargoyle to gather_switches/gather_flags or gather_feature_flags to make it not gargoyle specific and
a) test for gargoyle availability in the default implementation or b) do the actual implementation in the app that uses preflight (which knows whether they use gargoyle or something else)

- rename "Gargoyle" in l.91 to something more generic

- l.141-143 change the wording so that it's more generic and doesn't depend on gargoyle specifically (or place inside the {% if gargoyle %} so that we don't even show the section if it's empty)

- the django version check should be
django >= 1.4, django < 1.5

Unmerged revisions

22. By Michael Foord

Comment out currently failing test

21. By Michael Foord

Initial test for gather_gargoyle

20. By Michael Foord

Fix docs

19. By Michael Foord

Fix gargoyle switch rendering

18. By Michael Foord

Add links to sections to overview page

17. By Michael Foord

Correct import

16. By Michael Foord

Swapping django 1.0 for 1.4 in tox.
Initial implementation of gargoyle switch display.

15. By Michael Foord

Ignore wing project files

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

to all changes: