Re: python/dist/src/Lib rfc822.py,1.78,1.79
rhettinger@users.sourceforge.net wrote:
@@ -399,9 +393,8 @@ del self[name] # Won't fail if it doesn't exist self.dict[name.lower()] = value text = name + ": " + value - lines = text.split("\n") - for line in lines: - self.headers.append(line + "\n") + self.headers.extend(text.splitlines(True)) + self.headers.append('\n')
and you're 100% sure that the change in how things are stored in headers won't affect any existing code? (the docstring says that headers contain a list of lines, which is no longer true) </F>
@@ -399,9 +393,8 @@ del self[name] # Won't fail if it doesn't exist self.dict[name.lower()] = value text = name + ": " + value - lines = text.split("\n") - for line in lines: - self.headers.append(line + "\n") + self.headers.extend(text.splitlines(True)) + self.headers.append('\n')
and you're 100% sure that the change in how things are stored in headers won't affect any existing code?
(the docstring says that headers contain a list of lines, which is no longer true)
and the module documentation says: Each line contains a trailing newline. The blank line terminating the headers is not contained in the list. which is no longer true (unless I'm missing something here) </F>
On Tue, 8 Feb 2005 10:10:49 +0100, Fredrik Lundh <fredrik@pythonware.com> wrote:
@@ -399,9 +393,8 @@ del self[name] # Won't fail if it doesn't exist self.dict[name.lower()] = value text = name + ": " + value - lines = text.split("\n") - for line in lines: - self.headers.append(line + "\n") + self.headers.extend(text.splitlines(True)) + self.headers.append('\n')
and you're 100% sure that the change in how things are stored in headers won't affect any existing code?
(the docstring says that headers contain a list of lines, which is no longer true)
and the module documentation says:
Each line contains a trailing newline. The blank line terminating the headers is not contained in the list.
which is no longer true (unless I'm missing something here)
This would have been caught if there was a unit test validating what the documentation says. Why aren't there unit tests for this code? I think we need to raise the bar for "wholistic" improvements to a module: first write a unit test if there isn't already one (and if there is one, make sure that it tests all documented behavior), *then* refactor. Yes, this would be less fun. It's not supposed to be fun. It's supposed to avoid breaking code. Raymond, please roll back that change until this is taken care of. -- --Guido van Rossum (home page: http://www.python.org/~guido/)
On Tue, 2005-02-08 at 10:35, Guido van Rossum wrote:
This would have been caught if there was a unit test validating what the documentation says. Why aren't there unit tests for this code? I think we need to raise the bar for "wholistic" improvements to a module: first write a unit test if there isn't already one (and if there is one, make sure that it tests all documented behavior), *then* refactor. Yes, this would be less fun. It's not supposed to be fun. It's supposed to avoid breaking code.
+1. This module is used in so many place, you really have to take the documented interface seriously (not that you shouldn't otherwise, of course). I suspect even the undocumented current semantics are relied on in many place. -Barry
participants (3)
-
Barry Warsaw -
Fredrik Lundh -
Guido van Rossum