Giampaolo Rodola' added the comment:

> 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.

__________________________________
Tracker <[EMAIL PROTECTED]>
<http://bugs.python.org/issue1960>
__________________________________
_______________________________________________
Python-bugs-list mailing list 
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to