> On March 29, 2018, 5:07 a.m., Benjamin Bannier wrote: > > include/mesos/mesos.proto > > Lines 1920-1921 (patched) > > <https://reviews.apache.org/r/66049/diff/5/?file=1989744#file1989744line1920> > > > > These enum values are not handled in a number of switch statements in > > the code so that this patch cannot be used in isolation (fatal compiler > > warnings). It might be enough to enumerate the new cases explicitly and > > mark them `UNIMPLEMENTED`. > > > > Similar for v1 enum below. > > Zhitao Li wrote: > My plan was to commit all patches leading to proper full implementation > in one shot so code on master is still compilable (which seems previous norm > to me?) > > If you think marking them as `UNIMPLEMENTED` then gradually implement > each in later patches is better, I can happily do that. > > Greg Mann wrote: > We generally try to make each patch self-contained, so that the codebase > will still compile and tests will still pass if that patch were applied > alone. We do sometimes break from this when it's very difficult to > accomplish. I think that Benjamin's comment here is valid; I'll leave it up > to you what you want to do with this patch, but in the future, try to make > patches self-contained when possible.
That's fair. I like your proposal to keep patches self-contained if possible (just weren't clear on this practice). I'll add `UNIMPLEMENTED();` to broken enum checks in this patch. - Zhitao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66049/#review200185 ----------------------------------------------------------- On March 28, 2018, 11:24 a.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66049/ > ----------------------------------------------------------- > > (Updated March 28, 2018, 11:24 a.m.) > > > Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann. > > > Bugs: MESOS-4965 > https://issues.apache.org/jira/browse/MESOS-4965 > > > Repository: mesos > > > Description > ------- > > Added offer operation to grow and shrink persistent volumes. > > > Diffs > ----- > > include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 > include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 > > > Diff: https://reviews.apache.org/r/66049/diff/6/ > > > Testing > ------- > > > Thanks, > > Zhitao Li > >
