STINNER Victor <victor.stin...@haypocalc.com> added the comment: > See the patch attached.
I like your patch, it removes many duplicate code :-) Some comments: - I don't like an if surrounding the whole function, I prefer "if not ...: return" to avoid the (useless) indentation. Well, it's my coding style, do as you want. - You should add docstrings. Something like "Raise a SkipTest if the OS is Linux and the kernel version if lesser than min_version. For example, support.requires_linux_version(2, 6, 28) raises a SkipTest if the version is lesser than 2.6.28." - You should move the "if version < ..." outside the try/except ValueError + # platform.release() is something like '2.6.33.7-desktop-2mnb' + version_string = platform.release().split('-')[0] Dummy micro-optimization: .split('-', 1)[0] or .partition('-')[0] is maybe better than .split('-')[0]. > > By the way, I like the new os.pipe2() function! You may want to document > > it in > > the "What's new in Python 3.3" doc (just mention the new function, the > > document will be rephrased later). > > Sure, where is this document? Doc/whatsnew/3.3.rst ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue12196> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com