Message52472
Wow this patch is big!
I notice a lot of whitespace changes, can you drop those from the patch, it will make it easier to review. I don't see any extra whitespace in the original, so maybe if you ran Tools/scripts/reindent.py over the code that would clean it up. I think that's what Tim uses to normalize whitespace.
The new tests are good to see. I see you added doc for a new method in xmlparser, but the new class EntityReference from minidom.py is not documented.
In xml/dom/minidom.py, it seems odd to see if cond: pass else: # do something. What about inverting the condition so pass/else is not necessary. The comment could be improved here as to why this is a no-op.
The patch itself looked fine to me. I didn't review in complete context (ie, I didn't apply it and review all the code), just the patch itself. I don't know enough about minidom to know if this change it good or not. Other than what I noted above, the patch looks decent on the surface. |
|
| Date |
User |
Action |
Args |
| 2007-08-23 15:58:06 | admin | link | issue1704134 messages |
| 2007-08-23 15:58:06 | admin | create | |
|