D1033: pycompat: define operating system constants

spectral (Kyle Lippincott) phabricator at mercurial-scm.org
Thu Oct 12 15:31:13 EDT 2017


spectral accepted this revision.
spectral added a comment.


  [Accepted with a very mild concern; I'm fine with it not being addressed]

INLINE COMMENTS

> quark wrote in pycompat.py:24
> For encoding, Python 2 and 3 are compatible if we use `r` prefixed strings.
> 
> The `if` condition is to normalize strings to bytes for Mercurial use. In other words, I think only types that need to be "normalized" should be put under the `if` condition.
> 
> These `is*` constants have a type of `bool`, which does not need to be normalized. I think it's better to avoid code duplication.

Ah, sorry, I didn't mean put it in the if blocks (duplicating it), I meant just put it at the end of the file once <thismodule>.osname and <thismodule>.sysplatform have been created in a python-version-aware fashion, not indented at all.  (basically, move these three lines to the end of the file).

Mostly this is paranoia and a tiny extra layer of abstraction/indirection.  I don't expect it to happen, but if sys.platform renames itself in some hypothetical future python 4.2, I expect we'll continue to have a pycompat.sysplatform, and then we shouldn't need to change these lines at all, if they're using that version-agnostic sysplatform.

I don't feel strongly about this though, and don't want to block you on this issue.  I leave it up to you and the 2nd pass reviewers :)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1033

To: quark, #hg-reviewers, spectral
Cc: spectral, mercurial-devel


More information about the Mercurial-devel mailing list