Terry J. Reedy <tjre...@udel.edu> added the comment: Since we have defined an enum, I think we should use it consistently. (Am I correct in thinking it was added after the initial patches.) So I did a sanity check of -/+ lines for perhaps unintended changes. The only things I found to comment on, mostly somewhat picky, would *not* trigger test failures.
=== - int kind = PyUnicode_KIND(output); + kind = PyUnicode_KIND(output); extra space added misaligns text (for humans) === + default: assert(0); I presume this is standard elsewhere for switches and just got omitted here. - } - assert(0); - return -1; + default: + assert(0); + return -1; + } (two places) Since assert(0) always fails, return can never happen (and was not added above. So I would think remove it. And I agree with putting it consistently under default: (which is a form of else) even if not required as it improves human readability. === - int kind_self; - int kind_sub; + enum PyUnicode_Kind kind_self; + enum PyUnicode_Kind kind_sub; void *data_self; void *data_sub; Several places elsewhere, multiple enum vars are defined on one line, so you *could* do the same here (and with void *vars?). ==== - int kind1, kind2, kind; + enum PyUnicode_Kind kind1, kind2, kind; <four places> being really picky, I would put 'kind' first, not last, unless there is a good reason in the omitted context. It is mentally jarring for me as is without context. Elsewhere, things like src_/dest_/kind and x/y/z/kind are in the 'expected' order. ---------- nosy: +terry.reedy _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue15092> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com