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