martinvonz added inline comments. INLINE COMMENTS
> marmoute wrote in nodemap.py:47-49 > I don't really like `serialize` because we won't have a `deserialize` step. > The data should be usable directly with a mmap from disk. So if we > standardize function name on something I would rather go toward "persist" > (and "parse" for the python function that checks it). > > This can apply to the "dump" function and docstring. Unless the Python code is going to mmap the file contents directly into Python objects (which I don't think is going to happen), there will be serialization and deserialization involved. > martinvonz wrote in nodemap.py:66 > nit: maybe "... involution (is its own inverse)" because i don't think most > people know what an involution is > > Also, you could rename the function to `encode_rev()`. You could then do: > > # encode_rev is its own inverse > decode_rev = encode_rev > > Or not even define that since it's not used > I don't think it very important to define involution, people can look for it > if they need to. But it's helpful to be helpful, no? Is there a good reason *not* to try to help? Maybe I'm in a small minority who didn't know what an involution is? > marmoute wrote in nodemap.py:85 > We need to index the entry anyway, so that won't change the line much. I find > the current form clearer. I don't see why you'd be against `enumerate()` here. Are you simply against `enumerate()` in general? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7834/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7834 To: marmoute, #hg-reviewers, martinvonz Cc: martinvonz, mjpieters, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
