Message112949
(Apparently I don't have the right permissions on Rietveld.)
- Docstrings should be written in the standard PEP-8 way (single line
summary + additional explanation as needed following a blank line).
- read_sting and read_dict should still take a `filename` argument for
use in messages, with <string> and something like <data in ...> (with
the caller's __file__ being filled in if not provided).
- Indentation in the last read_dict of
test.test_cfgparser.BasicTestCase.test_basic_from_dict is
incconsistent with the previous read_dict in the same test.
- Lines over 79 characters should be shortened. Most of these are in
docstrings, so just re-wrapping should be sufficient for most.
- Changing the test structure to avoid self.cf may have been convenient,
but is unrelated to the actual functionality changes. In the future
such refactorings should be performed in separate patches. (Ordering
dependencies are fine, so long as they're noted in the relevant
issues.)
- DuplicateOptionError is missing from __all__.
- Changing the constructor to use keyword-only arguments carries some
backward-compatibility risk. That can be avvoided by removing that
change and adding strict=False at the end of the parameter list.
It's unlikely that this is a significant risk, since these parameters
generally lend themselves to keyword usage.
I think this should have been several separate patches:
- refactoring (the self.cf changes in the tests)
- addition of the DuplicateOptionError
- the read_* methods (including the readfp deprecation)
- the new "strict" option
Don't change that at this point, but please consider smaller chunks in
the future. |
|
Date |
User |
Action |
Args |
2010-08-05 07:10:07 | fdrake | set | recipients:
+ fdrake, ezio.melotti, brian.curtin, lukasz.langa |
2010-08-05 07:10:07 | fdrake | set | messageid: <1280992207.26.0.529487030266.issue9452@psf.upfronthosting.co.za> |
2010-08-05 07:10:05 | fdrake | link | issue9452 messages |
2010-08-05 07:10:04 | fdrake | create | |
|