Am 26.01.2011 10:57, schrieb Victor Stinner: > Hi, > > Le mardi 25 janvier 2011 à 18:07 -0800, Brett Cannon a écrit : >> This broke the buildbots (R. David Murray thinks you may have >> forgotten to call super() in the 'payload is None' branch). Are you >> getting code reviews and fully running the test suite before >> committing? We are in RC. >> (...) >> > - if _has_surrogates(msg._payload): >> > - self.write(msg._payload) >> > + payload = msg.get_payload() >> > + if payload is None: >> > + return >> > + if _has_surrogates(payload): >> > + self.write(payload) > > I didn't realize that such minor change can do anything harmful:
That's why the rule is that *every change needs to be reviewed*, not *every change that doesn't look harmful needs to be reviewed*. (This is true only for code changes, of course. Doc changes rarely have hidden bugs, nor are they embarrassing when a bug slips into the release. And I get the "test suite" (building the docs) results twice a day and can fix problems myself.) > the > parent method (Generator._handle_text) has exaclty the same test. If > msg._payload is None, call the parent method with None does nothing. But > _has_surrogates() doesn't support None. > > The problem is not the test of None, but replacing msg._payload by > msg.get_payload(). I thought that get_payload() was a dummy getter > reading self._payload, but I was completly wrong :-) > > I was stupid to not run at least test_email, sorry. And no, I didn't ask > for a review, because I thought that such minor change cannot be > harmful. I hope you know better now :) *Always* run the test suite *before* even asking for review. Georg _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
