consume: add Avro support #25
Conversation
| if body == nil { | ||
| return nil, fmt.Errorf("cannot decode schema id") | ||
| } | ||
| // TODO: cache the schema |
There was a problem hiding this comment.
TODO's are great, but I like them to be converted into issues, so that they can be scheduled and not forgotten when we don't regularly visit the code.
| var enc func([]byte) string | ||
| switch typ { | ||
| case "json": | ||
| return func(data []byte) (json.RawMessage, error) { |
There was a problem hiding this comment.
At this point I'd probably consider pushing the generation of this function (and the one like it below) out to named functions of their own, just because the switch statement is getting a bit long.
There was a problem hiding this comment.
True. I will do it in another PR.
There was a problem hiding this comment.
I tried to "simplify" the switch using a map with dedicated method per type: 0e4a242 If you think it's okay, I can propose the PR (it's based on this branch).
There was a problem hiding this comment.
Personally I don't think it's too unwieldy currently - not sure that splitting it up will help readability.
| var enc func([]byte) string | ||
| switch typ { | ||
| case "json": | ||
| return func(data []byte) (json.RawMessage, error) { |
There was a problem hiding this comment.
Personally I don't think it's too unwieldy currently - not sure that splitting it up will help readability.
gillesfabio
force-pushed
the
consume-avro-support
branch
from
ceaf6fd to
73dc2ee
Compare
consumecommandkt->hkt