[Updated] D10028: sidedata-exchange: add `wanted_sidedata` and `sidedata_computers` to repos

[marmoute (Pierre-Yves David)]

This revision now requires changes to proceed.
marmoute added inline comments.
marmoute requested changes to this revision.

INLINE COMMENTS

> bundle2.py:1813
>  
> +def format_remote_wanted_sidedata(repo):
> +    wanted = b""

Can you add a small docstring to clarify the semantic of the function ?

> changegroup.py:953
>          self._bundlecaps = bundlecaps
> +        self._remote_sidedata = remote_sidedata or set()
>          self._isshallow = shallow

If you want to test for None, explicitly test for None (`is None`). Otherwise you introduce subtle bugs for our future self.

> exchange.py:423-426
> +    for category in sorted(bundle2.read_remote_wanted_sidedata(pushop.remote)):
> +        for kind, computers in repo._sidedata_computers.items():
> +            if computers.get(category):
> +                break

Why do we need multiples layer of loop here . I would expect an API to return a the appropriate object for a categories and that's it.

So, what is going on here ?

> exchange.py:1628
>  
> +    for category in repo._wanted_sidedata:
> +        for kind, computers in repo._sidedata_computers.items():

Same question here.

REPOSITORY
  rHG Mercurial

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

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

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