kevincox added inline comments.
kevincox accepted this revision.
INLINE COMMENTS
> nodemap.rs:268
> + /// itself because of the mutable borrow taken with the returned `Block`
> + fn mutable_block(&mut self, idx: usize) -> (usize, &mut Block, usize) {
> + let ro_blocks = &self.readonly;
It looks like this return value could use some abstraction but maybe its best
to wait until there are more users?
> nodemap.rs:273
> + if idx < ro_len {
> + // TODO OPTIM I think this makes two copies
> + self.growable.push(ro_blocks[idx].clone());
I don't quite understand. You have the immutable copy and the mutable copy but
that is WAI right?
> nodemap.rs:306
> + // visit_steps cannot be empty, since we always visit the root block
> + let (deepest_idx, mut nybble, rev_opt) = visit_steps.pop().unwrap();
> + let (mut block_idx, mut block, mut glen) =
nybble is very vague. Is this deepest_nibble or something?
> nodemap.rs:504
> + node_from_hex(&format!("{:0<40}", hex)).unwrap()
> + }
> +
Should these be `#[cfg(test)]`? We can always remove it later if there is a
valid production use but it makes new users think twice.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7795/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7795
To: gracinet, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
