Page MenuHomePhabricator

rust-nodemap: building blocks for nodetree structures
ClosedPublic

Authored by gracinet on Jan 6 2020, 2:24 PM.

Details

Summary

This is similar to nodetreenode in revlog.c. We give it
a higher level feeling for ease of handling in Rust context
and provide tools for tests and debugging.

The encoding choice is dictated by our ultimate goal in this
series, that is to make an append-only persistent version of
nodetree: the 0th Block must be adressed from other Blocks.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gracinet created this revision.Jan 6 2020, 2:24 PM
gracinet updated this revision to Diff 19133.Jan 10 2020, 10:03 AM
kevincox accepted this revision.Jan 15 2020, 9:02 AM
kevincox added inline comments.
rust/hg-core/src/revlog/nodemap.rs
93

I would call these get and set.

112

@kevincox thanks for the review!

rust/hg-core/src/revlog/nodemap.rs
93

Yes, I suppose read and write feel like I/O. Will do.

112

Nice, thanks for the tip

gracinet updated this revision to Diff 19322.Jan 15 2020, 6:46 PM
gracinet marked 2 inline comments as done.Jan 15 2020, 6:55 PM
gracinet added inline comments.
rust/hg-core/src/revlog/nodemap.rs
112

So, that gives formatting with braces, hence for consistency I changed the block! macro, too.

I didn't keep the hexadecimal formatting, because it'd now lead to lots of \" making the tests less readable.

An upside of this is that it's now really consistent with block!. A downside is that someone using it for real debugging with input given in hexadecimal would presumably have to mentally convert hexadecimal nybbles to their decimal form.
It would have been a bit of a drag in the intiial development effort, but I don't think
that'll be a problem in the future: : either it'll be on small data or with diffrent tools anyway.

Just a few nits here, but it looks like we're expecting an update on this series anyway, so maybe you can address them.

rust/hg-core/src/revlog/nodemap.rs
9

i think s/valency/arity/ is a more common term for it

37

s/endianity/endianness/ (same further down)

97

Could you call it element instead? I think it's a little obvious what that means. elt is at least not an abbreviation I'm familiar with.

@martinvonz

Just a few nits here, but it looks like we're expecting an update on this series anyway, so maybe you can address them.

No problem with these. I'll be doing a swipe on the whole series today and will include them.

rust/hg-core/src/revlog/nodemap.rs
97

right, I've must have spent too much of my life using XML libraries, I suppose ;-)

gracinet added inline comments.Jan 22 2020, 9:53 AM
rust/hg-core/src/revlog/nodemap.rs
9

yes, and valency is actually almost incorrect actually in that case, it turns out

gracinet marked 3 inline comments as done.Jan 22 2020, 10:34 AM
gracinet updated this revision to Diff 19503.

These are done, thanks for the remarks.

gracinet updated this revision to Diff 19504.Jan 22 2020, 10:42 AM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.