D6389: rust-dirstate: create dirstate submodule

kevincox (Kevin Cox) phabricator at mercurial-scm.org
Fri May 17 09:37:34 EDT 2019


kevincox added inline comments.

INLINE COMMENTS

> mod.rs:6
> +    pub p1: &'a [u8],
> +    pub p2: &'a [u8],
> +}

If 1 and 2 are the best names why not just make it `pub struct DirstateParetens([&[u8]; 2])`?

> mod.rs:7
> +    pub p2: &'a [u8],
> +}
> +

It seems odd that this struct is public seeing as it is just a bag of bytes. Would it make sense to keep this internal to the parser/serializer?

> parsers.rs:40
> +
> +        let mut cursor = Cursor::new(entry_bytes);
> +        let state = cursor.read_i8()?;

It seems to me that this code would be simpler if you constructed a single cursor at the top of the function and just used it to track your state instead of having a couple of other variables to track it.

REPOSITORY
  rHG Mercurial

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

To: Alphare, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel


More information about the Mercurial-devel mailing list