Author nnorwitz
Recipients
Date 2007-04-24.06:58:14
SpamBayes Score
Marked as misclassified
Message-id
In-reply-to
Content
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.
History
Date User Action Args
2007-08-23 15:58:06adminlinkissue1704134 messages
2007-08-23 15:58:06admincreate