> On Nov. 20, 2016, 9:04 p.m., Anand Mazumdar wrote: > > Nice first patch and welcome to the community! > > > > - Can you update the Testing Done section with details on testing? > > - We also support Python bindings. Do you mind adding these protos to our > > python build too in a follow up review? (See my comments later on how you > > can use review dependencies if you are new to ReviewBoard) > > Vijay Srinivasaraghavan wrote: > Thanks Anand. I have made the python binding changes locally on my > machine. Do you want me to create a new JIRA bug for this issue and create a > new review or add the patch to this JIRA/review itself? I am little confused > as to how the dependencies are handled with RB.
No need to create a separate issue. It would be fine to create a new review with it only containing the python related changes. This would be another commit based on top of this one. You can then use `post-reviews.py` and it should handle setting the dependencies for you. Feel free to ping me on IRC/Slack if you run into any issues (anandm) > On Nov. 20, 2016, 9:04 p.m., Anand Mazumdar wrote: > > include/mesos/v1/allocator/allocator.proto, lines 21-22 > > <https://reviews.apache.org/r/53825/diff/2/?file=1566692#file1566692line21> > > > > These changes seem unrelated to this change i.e., java protos > > generation. We prefer single logical atomic commits in Mesos. For more info > > see: http://mesos.apache.org/documentation/latest/submitting-a-patch/ > > > > Can you create a separate patch for this and make this review dependent > > on it? The `post-reviews.py` script would do it automatically for you. > > Vijay Srinivasaraghavan wrote: > Actually, this fix is also part of the same issue. The proto file was > missing specific java package definition and hence the java file gets > generated in a package location different from the other java files. hmm, are they the same? They are two separate issues: 1. The proto file was missing specific java package definition. This would impact any user trying to use the proto file and is not related to including the generated files in the Mesos JAR. 2. Include the generated files in the Mesos JAR. This review (from its summary/description) caters to _only_ build changes in the MakeFile itself and not protobuf related changes. - Anand ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53825/#review156423 ----------------------------------------------------------- On Nov. 21, 2016, 4:46 a.m., Vijay Srinivasaraghavan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53825/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2016, 4:46 a.m.) > > > Review request for mesos, Anand Mazumdar and Zameer Manji. > > > Bugs: MESOS-6597 > https://issues.apache.org/jira/browse/MESOS-6597 > > > Repository: mesos > > > Description > ------- > > MESOS-6597 Enabled java protos generation for all V1 proto files. > > > Diffs > ----- > > include/mesos/v1/allocator/allocator.proto > 73d45b37a7afc47366a4a01a36912f30b47c30b1 > src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac > > Diff: https://reviews.apache.org/r/53825/diff/ > > > Testing > ------- > > Built mesos and confirmed jar file containing new java PB wrappers for V1 > API. Ran standard unit test (make tests) > > > Thanks, > > Vijay Srinivasaraghavan > >
