search

protobuf-ruby / beefcake

A sane Google Protocol Buffers library for Ruby
MIT License
276 stars 46 forks source link

Code Generator produces messages with broken dependencies #28

Closed bkerley closed 11 years ago

bkerley commented 11 years ago

This is part of the output of running the generator from 67f4894e4a77bac6e3b5953ca51f2de12127fff7 against https://github.com/basho/riak_pb/blob/master/src/riak_kv.proto :


class RpbContent
  include Beefcake::Message

  required :value, :bytes, 1
  optional :content_type, :bytes, 2
  optional :charset, :bytes, 3
  optional :content_encoding, :bytes, 4
  optional :vtag, :bytes, 5
  repeated :links, RpbLink, 6
  optional :last_mod, :uint32, 7
  optional :last_mod_usecs, :uint32, 8
  repeated :usermeta, RpbPair, 9
  repeated :indexes, RpbPair, 10
  optional :deleted, :bool, 11

end

class RpbLink
  include Beefcake::Message

  optional :bucket, :bytes, 1
  optional :key, :bytes, 2
  optional :tag, :bytes, 3

end

The issue is that RpbContent.links cannot be defined, because it depends on the RpbLink class that hasn't been read yet.

What I'll try and do this week is find a way to create all the classes up front, so they all exist when they try to get tagged in messages.

bkerley commented 11 years ago

This includes and supersedes #27 .

matttproud commented 11 years ago

Hi, @bkerley! @bmizerany and I spoke a week ago about maintainership, and we have transitioned Beefcake to the @protobuf-ruby group on GitHub.

I would like to review your pull request, but it looks out of date. Can you rebase?

matttproud commented 11 years ago

Hey, so overall this looks OK. Would you be willing to include inline a before-and-after of the generated code or include them in Gist for review? I would like to compare this a bit more precisely.

Without having the diff, I am curious, why not output the messages with a topological sort instead of defining the class specification a priori?

bkerley commented 11 years ago

Just force-pushed the rebase, will generate before & after.

bkerley commented 11 years ago

https://gist.github.com/bkerley/6284225 contains before and after. The issue this branch fixes is at https://gist.github.com/bkerley/6284225#file-before-riak_kv-pb-rb-L221

bkerley commented 11 years ago

Taking a look at the jruby failure on travis: https://travis-ci.org/protobuf-ruby/beefcake/jobs/10417342

matttproud commented 11 years ago

Just a few open questions in https://github.com/protobuf-ruby/beefcake/pull/28/files.

matttproud commented 11 years ago

Almost ready for submission. Just take care of the last simple remarks, and we're good to merge.

matttproud commented 11 years ago

LGTM. Thank you for your work and patience and thoroughness!