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 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel