Serhiy Storchaka added the comment:
Adding new parameter in the middle of positional argument list can break
existing code. If you want change arguments order, make their keyword-only.
--
nosy: +serhiy.storchaka
___
Python tracker
Milan Oberkirch added the comment:
Thanks for your review comments, serhiy.storchaka!
I may be blind right now, but where did I add a positional parameter?
--
Added file: http://bugs.python.org/file36357/issue21725v5.4.patch
___
Python tracker
Milan Oberkirch added the comment:
Sorry, I was blind (switching between languages to much)! Anyway there should
be no existing code using decode_data as it was introduced in this development
circle.
--
___
Python tracker rep...@bugs.python.org
Roundup Robot added the comment:
New changeset ca696ca204e0 by R David Murray in branch 'default':
#21725: Add RFC 6531 (SMTPUTF8) support to smtpd.
http://hg.python.org/cpython/rev/ca696ca204e0
--
nosy: +python-dev
___
Python tracker
R. David Murray added the comment:
I tweaked your additions. Instead of trying to strip out the 'b' to make
things look pretty, I think it is better to print them and thus make it
explicit when we are dealing with binary data and when we are dealing with
strings. It also clues the user in
Milan Oberkirch added the comment:
I think that the peer arg is supposed to be set to the address of the peer
connecting to our server.
The value 'peer' comes from test/mock_socket.py:105 and is the best answer we
can get for mock_sock.getpeername() because there is no real client when
R. David Murray added the comment:
OK, it's a bug in mock_socket, then. getpeername should be returning a tuple
(address, port).
I went ahead and fixed it, and committed the patch.
Thanks Milan! Sorry the reviews were so delayed.
--
resolution: - fixed
stage: patch review -
Changes by Milan Oberkirch milan...@oberkirch.org:
Added file: http://bugs.python.org/file36323/issue21725v5.2.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
R. David Murray added the comment:
You can use test.support.captured_stdout or whatever its name is.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
Changes by Milan Oberkirch milan...@oberkirch.org:
Added file: http://bugs.python.org/file36157/issue21725v5.1.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
Milan Oberkirch added the comment:
Thanks for the review and improvements!
I fixed the warnings (didn't see them with running tests as described in the
dev guide; I may improve that later), updated the docs to RFC 5321 (issue 19679
is referring to RFC 3463 which is not
R. David Murray added the comment:
I have updated your patch, changing wording of some of the documentation, and
applying style tweaks to the code. I have also done the refactoring of the
_set_xxx_state method that I suggested. Seemed easier to show you what I meant
in code rather than try
R. David Murray added the comment:
Review comments added.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
___
Python-bugs-list mailing
Milan Oberkirch added the comment:
I replied to some review comments, and made a new patch implementing your
suggestions.
--
Added file: http://bugs.python.org/file35679/issue21725v3.patch
___
Python tracker rep...@bugs.python.org
Milan Oberkirch added the comment:
The new patch implements what you suggested, with the following differences:
- I still use the default_dict and add the numbers up but maybe it's a bit more
readable now?
- Support for HELO/EHLO is a seperate issue #21783
--
Added file:
Milan Oberkirch added the comment:
Thanks a lot for your feedback!
Your modifications to the handling of max_command_size are technically buggy
(and clearly there's no test for it): when we aren't in extended smtp mode,
the limit should be the hardcoded value, as in the original code. My
R. David Murray added the comment:
For some reason the diff shown by the review link is very different from the
one show by the patch file itself. I'm not sure what is causing that, since
the diff appears to be in the correct hg format. I don't even know where
reitveld is getting the stuff
R. David Murray added the comment:
Correction on the XXX should we check this: I was thinking about the wrong
section of the code. But it is still 'no': by postel's law we should accept
dirty data. Currently the consumer of the library can then decide whether or
not to reject the dirty data
New submission from R. David Murray:
I thought there was already an issue for this, but I can't find it. This is
part of this summer's GSOC work, and involves adding support for the SMTPUTF8
extension to smtpd.
--
components: email
messages: 220285
nosy: barry, jesstess, pitrou,
Changes by R. David Murray rdmur...@bitdance.com:
--
hgrepos: +256
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
___
Python-bugs-list
Changes by R. David Murray rdmur...@bitdance.com:
--
keywords: +patch
Added file: http://bugs.python.org/file35575/80ea1cdf2b23.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
R. David Murray added the comment:
Milan: your patch looks good for the most part. Now that I committed the
decode_data patch you should modify it so that SMTPUTF8 implies
decode_data=False (otherwise we *don't* have an 8-bit-clean channel). Please
either attach the modified patch here or
Changes by R. David Murray rdmur...@bitdance.com:
--
hgrepos: -256
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
___
Python-bugs-list
Changes by R. David Murray rdmur...@bitdance.com:
Removed file: http://bugs.python.org/file35575/80ea1cdf2b23.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
Milan Oberkirch added the comment:
I merged it into https://bitbucket.org/zvyn/cpython and made a diff from that.
(I used bash for this so hopefully its compatible to the review system here.)
--
Added file: http://bugs.python.org/file35577/issue21725.patch
Changes by Milan Oberkirch milan...@oberkirch.org:
Added file: http://bugs.python.org/file35578/issue21725-fixed.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
Changes by Milan Oberkirch milan...@oberkirch.org:
Removed file: http://bugs.python.org/file35578/issue21725-fixed.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
Changes by Milan Oberkirch milan...@oberkirch.org:
Added file: http://bugs.python.org/file35579/issue21725-fixed.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
Changes by Milan Oberkirch milan...@oberkirch.org:
Added file: http://bugs.python.org/file35580/issue21725-fixed-hg.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21725
___
R. David Murray added the comment:
Hmm. There's a conflict marker in that patch.
I *think* that if you do an 'hg pull --rebase' and then give your repository
URL to the tracker, the 'create patch' button will do the right thing. (I
tried it with the URL you sent me and it generated a diff
30 matches
Mail list logo
