Review feedback from Brad Dietrich

[maximecb]

Just copying over some of the feedback from @braddietrich in the ad-hoc review so that we don't lose track of it.

Bug in cross package imports: It generated the following ruby without the modules names separated by ::

@weight_unit =
  shopify.common.WeightUnit.allocate.decode_from(
    buff,
    index,
    index += msg_len
  )

Probably not relevant if we are using zeitwerk, but in one of my prototypes, I am not and I import the base object (e.g. product_pb.rb) which with protoc generated code includes the requires for the imported proto files but protoboeuf currently does not.

enum semantics: in code generated by protoc is different than code generated by protoboeuf (potentially for the positive). The specific case I am noticing is that setting an enum field to a symbol is causing _encode to fail because it is expecting it to be an integer. I haven't checked the sorbet specifically to see if it would have caught/forced this as my existing code was using the protoc generated version. I notice that the enum modules generated do appear to have the ability to map between the symbol and the integer, but I wouldn't expect to have to set the field by calling this myself.

@tenderworks should we be handling the case of setting an enum field as a symbol?

[cocoahero]

Bug in cross package imports: It generated the following ruby without the modules names separated by ::

This bug is further complicated by the ruby_package option, as the parser/codegen currently doesn't walk and parse any imported files. Once #113 ships, simply replacing . with :: will not work.

[tenderlove]

I think this bug is fixed in main. @rwstauner isn't this the one you fixed?

[rwstauner]

Yes, we were previously generating a.b.C.allocate in some cases and that should be fixed as A::B::C.allocate on main.

[rwstauner]

Setting/getting enums as symbols is also on main.