Changes between Initial Version and Version 2 of Ticket #6017


Ignore:
Timestamp:
Nov 26, 2007, 7:07:45 AM (17 years ago)
Author:
Russell Keith-Magee
Comment:

The patch itself looks good; my quick poking didn't reveal any problems, but that's just quick poking. Since you worked out a way to regression test this bit, it should be a bit more comprehensive than one test. There are several branch paths in the patch - these should all be tested (at the very least, -X , --X and --X=Y option types should all be checked, in various orders and combinations with --settings etc).

Also, a minor point of style - calling the test directory 'management' is sufficient.

I've attached a slightly revised patch, with a couple more tests and cleaned up comments; if you can bulk out the patch with some more tests, I'll commit.

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #6017

    • Property Component Uncategorized → Core framework
    • Property Has patch set
    • Property Needs tests set
    • Property Triage Stage Unreviewed → Accepted

  • Ticket #6017 – Description

    initial v2  
    11In ticket #5369, ``django-admin.py`` and ``manage.py`` are supposed to read all options, looking for ``--settings`` and ``--pythonpath``, and then use those two if they're present to access app-supplied commands. The code (which was mine) has a bug, in that options stop getting read as soon as something other than ``--settings`` or ``--pythonpath`` gets hit.
    22
    3 Without the patch, you currently get: {{{
     3Without the patch, you currently get:
     4{{{
    45>>> from django.core.management import LaxOptionParser, get_version
    56>>> from django.core.management.base import BaseCommand
    … …  
    1314where both ``--pythonpath`` and ``--settings`` get ignored.
    1415
    15 With the included patch, you get:{{{
     16With the included patch, you get:
     17{{{
    1618>>> from django.core.management import LaxOptionParser, get_version
    1719>>> from django.core.management.base import BaseCommand
Back to Top