kevincox added inline comments.
kevincox accepted this revision.
INLINE COMMENTS
> mod.rs:16
> + pub p1: Vec<u8>,
> + pub p2: Vec<u8>,
> }
If these are always 20 bytes long you are best to use `[u8; 20]`. That way
there will be no indirection. And since `Vec` is 3 words it will have a bigger
immediate size on 64bit platforms.
> parsers.rs:81
> parents: DirstateParents,
> - now: i32,
> -) -> Result<(Vec<u8>, DirstateVec), DirstatePackError> {
> + now: Duration,
> +) -> Result<Vec<u8>, DirstatePackError> {
The "proper" solution is SystemTime
<https://doc.rust-lang.org/std/time/struct.SystemTime.html> however I will
admit that it is a bit more awkward to use. However if you are going to use
Duration please document that it is the Duration since the Unix Epoch.
> parsers.rs:87
>
> - let expected_size: usize = dirstate_vec
> + let now = now.as_secs() as i32;
> +
Would it be best to panic on overflow?
`now.as_secs().try_into::<i32>().expect("time overflow")`.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D6628/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D6628
To: Alphare, #hg-reviewers, kevincox
Cc: durin42, kevincox, mjpieters, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
