[Commented On] D8928: worker: don't expose readinto() on _blockingreader since pickle is picky

marmoute (Pierre-Yves David) phabricator at mercurial-scm.org
Wed Aug 26 16:12:51 UTC 2020


marmoute added a comment.
marmoute accepted this revision.


  This looks overall good. However I recommend adding a comment to make this does not regress (see inline comment)

INLINE COMMENTS

> worker.py:75
> +        def readline(self):
> +            return self._wrapped.readline()
>  

Can we add a comment the explain why `readinto` is missing from the object signature? That would avoid people re-introducing it carelessly in the future.

> worker.py:94
>              del buf[pos:]
> -            return buf
> +            return bytes(buf)
>  

This seems like a correct, but unrelated change. Should we extract it in its own changesets?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8928/new/

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

To: martinvonz, #hg-reviewers, marmoute
Cc: marmoute, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200826/ded3c3d5/attachment-0002.html>


More information about the Mercurial-patches mailing list