attic-labs / noms

The versioned, forkable, syncable database
Apache License 2.0
7.44k stars 266 forks source link

Remove in mem graphs #3635

Closed ghost closed 7 years ago

ghost commented 7 years ago

Hey @arv,

This patch isn't complete. There's still a bunch of mechanical changes to be made that are fall out of requiring Collections to have a ValueReadWriter upon creation.

I was hoping to get this finished today, but I ran out of time and tomorrow I won't have a ton of time. I know that your lazy decode patch is blocked on this, so I wanted to give you option of taking this over.

The number of files touched (already) is very large, but the interesting code changes here are deeply un-interesting. It's basically just making sequences have a ValueReadWriter (as opposed to ValueReader), making that required (as opposed to nil-able), and then removing MetaTuple.child. The rest is just fixing up-stream compile errors.

Don't feel obliged, but if you feel like taking the baton, it'd be helpful

arv commented 7 years ago

Jenkins: test this again

arv commented 7 years ago

Jenkins: test this again

arv commented 7 years ago

Hmm... I cannot reproduce the failing test locally. Not clear why it does not repro here.

arv commented 7 years ago

Jenkins: Test this to see if this was flake or whatever the f it was...

arv commented 7 years ago

jenkins: test this

arv commented 7 years ago

@rafael-atticlabs PTAL

I ended up skipping perf/suite.go@TestRunFlag. I'll file an issue when this is ready.

arv commented 7 years ago

jenkins: test this

arv commented 7 years ago

PTAL

Fixed the merge conflicts and removed the logging