[Updated] D8654: phases: make phase list dense or dictionaries [PoC]

[joerg.sonnenberger (Joerg Sonnenberger)]

joerg.sonnenberger added inline comments.

INLINE COMMENTS

> mjacob wrote in exchangev2.py:85
> This should probably use `itervalues()`, but see below.

phasenames contains a small number of strings, the copy/reference doesn't matter here.

> mjacob wrote in exchangev2.py:92
> If I understand correctly, `phases.phasenumber` is the same as the key for `phase` in this for loop. If you use `iteritems()` above, I think `phases.phasenumber` is unused.

Right, possible. This was meant to preserve the original logic for now, but I'll update it for the next iteration.

> mjacob wrote in exchangev2.py:364
> This should probably use `itervalues()` (not that it makes a difference for the small dict).

Yeah, I don't think it makes a difference at all and it is more verbose.

> mjacob wrote in phases.py:133-134
> Is it for backwards compatibility? It wasn’t obvious for me without a comment.

Yes, both phases can be seen in existing repositories, so the values have to be preserved. Otherwise I would have moved them to 3 and 4, which would have greatly simplified things.

REPOSITORY
  rHG Mercurial

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

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

To: joerg.sonnenberger, #hg-reviewers
Cc: mjacob, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200624/e91206ed/attachment-0002.html>