Serhiy Storchaka added the comment: > (Not having side by side diffs as with Rietveld makes this much harder.)
Click on the "Files changed" tab on the PR page: https://github.com/python/cpython/pull/825/files . You can add inline comments when click on the "+" button that follows your mouse cursor. After entering the comment press the GREEN "Start a review" button rather than "Add single comment". This will allow to send all your comments at once rather than sending them as separate emails, and will allow to edit or remove comments before sending them. After reviewing all changes press the GREEN button at the top right corner of the page for sending your comments. Yes, it is my fault that I missed the conflict. But the user can add conflicting shortcuts for other events, so it would be better to make them safe even if there re not conflicts in standard configuration. > In some places, you add "return None" instead. PEP 8: "Be consistent in return statements. Either all return statements in a function should return an expression, or none of them should. If any return statement returns an expression, any return statements where no value is returned should explicitly state this as return None , and an explicit return statement should be present at the end of the function (if reachable)." > If there is a masked binding, having 'key-x' do one thing sometimes and > something else other times would seem disconcerting. I think this is okay. In the specific context one this is done, but if this context does not exist fall back to doing other thing. I don't think special tests are needed. There are too much event handlers, and testing them with monkey-patching will just complicate the code and will not check anything beside the fact that that event handlers are written in special style. ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue29910> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
