> On June 12, 2017, 11:41 p.m., Zhitao Li wrote: > > It seems that majority of this patch is generated code. Is the `.proto` > > change the only real code change? If so, should we write some c++ test code > > to use the map based fields and json parsing of map? > > > > Also, explaining which files are generated in summary will help reviewers > > to know what changes we can safekly skip reading? > > Qian Zhang wrote: > Yes, the `.proto` change is the only real code changed, I have updated > the description to mention that. And for the test, please refer to > https://reviews.apache.org/r/59989/. > > Benjamin Bannier wrote: > Could you have a look and try to find out how hard it would be to > generate the `*.pb.*` files during the build instead of checking them in, > https://issues.apache.org/jira/browse/MESOS-6115? > > Qian Zhang wrote: > In MESOS-6115, I see you mentioned: > > We should try to remove them from the source tree; in fact > protobuf_tests.cpp already documents ways this could be achieved. > > I am not sure what you meant for `protobuf_tests.cpp already documents > ways this could be achieved.`, did you mean the comments here: > https://github.com/apache/mesos/blob/1.3.0/3rdparty/stout/tests/protobuf_tests.cpp#L88:L104 > which seems the way to generate protobuf message dynamically in the runtime > rather than during the build. > > Benjamin Bannier wrote: > I am not sure what I was referring to there, so I removed that line from > the ticket. It seems just adding a rule to generate these output files and > adding them to the test binary like we already do would do the job.
OK, I have posted a patch (https://reviews.apache.org/r/60109/) to generate `protobuf_tests.pb.*` during the build. I am not very familar with automake, so although the patch works, it may not be the right solution, please help review and let me know for any comments, thanks! - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59988/#review177626 ----------------------------------------------------------- On June 13, 2017, 4:41 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59988/ > ----------------------------------------------------------- > > (Updated June 13, 2017, 4:41 p.m.) > > > Review request for mesos, Anand Mazumdar and Zhitao Li. > > > Repository: mesos > > > Description > ------- > > The real code changes is adding `MapMessage` to protobuf_tests.proto, > for the other two files in this patch, they are automatically generated > by running protobuf-3.3.0 compiler `protoc` on protobuf_tests.proto. > > > Diffs > ----- > > 3rdparty/stout/tests/protobuf_tests.pb.h > 2e4ffe17a07ce2360ec618e936ae4557e9dc8e62 > 3rdparty/stout/tests/protobuf_tests.pb.cc > ad6eff779d1cc0e7d037ea77565533c3ebb0b2d6 > 3rdparty/stout/tests/protobuf_tests.proto > d16726aa8060aea2b830040b20dbdd467c801483 > > > Diff: https://reviews.apache.org/r/59988/diff/2/ > > > Testing > ------- > > > Thanks, > > Qian Zhang > >
