Author giampaolo.rodola
Recipients brett.cannon, giampaolo.rodola
Date 2008-01-29.02:20:15
SpamBayes Score 0.00190646
Marked as misclassified No
Message-id <1201573218.16.0.448704306744.issue1960@psf.upfronthosting.co.za>
In-reply-to
Content
> Well, if you have an opinion, feel free to leave a comment in this
> issue about it. I will most likely be the one who does the checkin and
> I will read this issue before I commit.

 * One of the things I dislike is the fact that the student used "self.g
= gdbm.open(self.filename, 'c')" in setUp method since if that fails
there will be a NameError exception raised in tearDown method and the
errors reported by unittest would be 2 where in fact it would be only 1.
Probably a thing like this one would have been better:

    def setUp(self):
        self.g = None
    
    def tearDown(self):
        if self.g is not None:
            self.g.close()
	  ...

    def test_it(self):
        self.g = gdbm.open(self.filename, 'c')
        ...

- Another thing I don't like is that os.unlink(self.filename) executed
in tearDown would be better if protected by a try/except statement.

- +1 for the self.g.close() used by the student in the tearDown method.
That was a thing I haven't considered and it's not included in my patch.

- +0.5 for the "test_modes" method added by the student. Maybe it's
useful, maybe it's not, I don't know. 


Aside from that I don't see other relevant differences since the code is
really minimalistic. Feel free to commit the patch you consider to be
the best. 
If you want me to do so I can provide a merged version of my patch
including the differences I described above.
History
Date User Action Args
2008-01-29 02:20:18giampaolo.rodolasetspambayes_score: 0.00190646 -> 0.00190646
recipients: + giampaolo.rodola, brett.cannon
2008-01-29 02:20:18giampaolo.rodolasetspambayes_score: 0.00190646 -> 0.00190646
messageid: <1201573218.16.0.448704306744.issue1960@psf.upfronthosting.co.za>
2008-01-29 02:20:16giampaolo.rodolalinkissue1960 messages
2008-01-29 02:20:15giampaolo.rodolacreate