martinvonz added inline comments.

INLINE COMMENTS

> shelve.py:61
> +_pack = struct.pack
> +_unpack = struct.unpack
> +

Seems unused

> shelve.py:73-112
> +# Merge state record types. See ``mergestate`` docs for more.
> +RECORD_LOCAL = b'L'
> +RECORD_OTHER = b'O'
> +RECORD_MERGED = b'F'
> +RECORD_CHANGEDELETE_CONFLICT = b'C'
> +RECORD_MERGE_DRIVER_MERGE = b'D'
> +RECORD_PATH_CONFLICT = b'P'

Most of these seem unused. I would prefer if they were added in a later patch 
where they are used

> shelve.py:475
> +            key, data = RECORD_OVERRIDE, '%s%s' % (key, data)
> +        format = '>sI%is' % len(data)
> +        mergedata = ''.join([mergedata, _pack(format, key, len(data), data)])

need a `r''` prefix here for py3?

> shelve.py:476
> +        format = '>sI%is' % len(data)
> +        mergedata = ''.join([mergedata, _pack(format, key, len(data), data)])
> +    extra['mergerecords'] = mergedata

`''.join(a,b)` can be written more simply as `a + b`

> shelve.py:478
> +    extra['mergerecords'] = mergedata
> +    ms.reset()
> +

This seems surprising in this method. Consider moving it out.

REPOSITORY
  rHG Mercurial

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

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

To: navaneeth.suresh, #hg-reviewers
Cc: martinvonz, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[email protected]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to