Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @karampok and @oncilla)

go/proto/cereal.go, line 22 at r1 (raw file):

	"io"

	capnp "zombiezen.com/go/capnproto2"

Seems that is being added by my go-import automatically.
Do you understand why?

go/proto/cereal.go, line 113 at r1 (raw file):

		return err
	}
	return parseStruct(c, s)

Do we use this function? if not maybe delete the implementation

go/proto/cereal_test.go, line 61 at r1 (raw file):

			a := &asEntry{}
			raw := make(common.RawBytes, 1024)
			n, err := WriteRoot(a, raw)

Ideally your unit test on function ParseFromRaw should not depend on any other function within the file. Maybe you could hardcode the raw (or maybe rewrite the help-testing function)

go/proto/cereal_test.go, line 67 at r1 (raw file):

				err = ParseFromRaw(a, raw)
			}
			pogsExtractF = test.Extractor

This is confusing to me, I would like more to have the ParseFromRaw to accept another argument (the function pogsExtractF) for readability. But it is okay to let it like that

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @oncilla)

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @karampok)

go/proto/cereal.go, line 113 at r1 (raw file):

Good point.
Done.

go/proto/cereal_test.go, line 61 at r1 (raw file):

It is a bit hard to hard-code this.
If we change the proto file for ASEntry, then the unit tests will suddenly fail, which is unexpected.
Also, if WriteRoot does not work as expected, we are in a completely different world of pain :)

go/proto/cereal_test.go, line 67 at r1 (raw file):

Yeah. I also don't like the package variable that defines of stuff is extracted.
But I think it was the only way to make this testable without having an API that you suggested.

Note, in normal operation it does not make sense to provide any other extractor function, this is truly just for testing the code and the API exposed by ParseFromRaw is the correct one.

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @oncilla)

go/proto/cereal_test.go, line 61 at r1 (raw file):

In theory you could use once the WriteRoot and save to a buffer var the value you need (assuming that now WriteRoot currently works).
I would prefer that mainly because of keeping the files short (fewer lines of code).
But up to you, not strong feeling.

go/proto/cereal_test.go, line 67 at r1 (raw file):

fair

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @oncilla)

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @karampok)

go/proto/cereal_test.go, line 61 at r1 (raw file):

Done.

go/proto/cereal_test.go, line 67 at r1 (raw file):

Done.

Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved