mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zhitao Li <zhitaoli...@gmail.com>
Subject Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.
Date Thu, 12 Apr 2018 21:45:05 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message