Author alexandre.vassalotti
Recipients alexandre.vassalotti, collinwinter
Date 2009-04-03.03:46:23
SpamBayes Score 1.61786e-06
Marked as misclassified No
Message-id <1238730386.72.0.957471991058.issue5665@psf.upfronthosting.co.za>
In-reply-to
Content
Overall, the patch looks good. I haven't reviewed the patch thoroughly
yet, but there is a few things I am not sure about. First, why do you
bother supporting 2.4 (i.e. with a copy of run_with_locale) in a patch
aimed at 2.7? 

In test_many_puts_and_gets(), I believe this:

  for proto in [0, 1, 2]:
    ...

should be changed to:

  for proto in protocols:
    ...

Also, I think the tests in AbstractPicklerUnpicklerObjectTests could be
stronger if they used the pickletools module to decompile to check that
only PUT opcodes have changed or not. However, the extra complexity may
not worth it.

And one last thing, why AbstractCompatTests is hard-coded to use
cPickle? Shouldn't the standard pickle module be tested too?
History
Date User Action Args
2009-04-03 03:46:27alexandre.vassalottisetrecipients: + alexandre.vassalotti, collinwinter
2009-04-03 03:46:26alexandre.vassalottisetmessageid: <1238730386.72.0.957471991058.issue5665@psf.upfronthosting.co.za>
2009-04-03 03:46:25alexandre.vassalottilinkissue5665 messages
2009-04-03 03:46:24alexandre.vassalotticreate