Message49353
I've looked at the patches attached here. They look reasonable, and in isolation I would be happy for them to be accepted, although I have no personal use for the functionality.
However, patch 131075 (ConfigParser to accept a custom dict to allow ordering) has since been accepted, and in the light of that, this patch may no longer be appropriate. From the descriptions, and a review of the code, I am not clear how the two are related. The justification should be updated to reflect the fact that patch 131075 has been accepted - or if there is no longer sufficient justification for what may well be a second way to do the same thing, then this patch should be withdrawn.
I have looked at Scott Dial's updated patches, but I am not clear on what they add. There has obviously been some discussion which happened off the tracker - as a result I can't comment on the DEFAULTSECT issue. The whitespace stripping issue needs a clearer description. I don't see what is happening here. One or other of the patches is presumably mishandling leading or trailing whitespace in options, but I can't tell which. The "growing blank lines" fix sounds sensible.
Recommendation:
1. The OP should review the justification in the light of the acceptance of patch 1371075.
2. Scott should clarify the issues around the 2 areas I mentioned.
3. An updated patch should be submitted, either by the OP against this tracker, or by someone else (Scott, perhaps) under a new tracker item, pointing back to this one.
Otherwise, I would recommend rejection on the grounds that the functionality now appears to be available via the acceptance of 1371075 (albeit in a form that this patch claims is inferior).
Paul Moore |
|
Date |
User |
Action |
Args |
2007-08-23 15:45:23 | admin | link | issue1410680 messages |
2007-08-23 15:45:23 | admin | create | |
|