Re: Review Request 57911: WIP: Added UNKNOWN DiskInfo.Source type.

[Benjamin Bannier]

> On March 28, 2017, 7:44 p.m., Jie Yu wrote:
> > src/slave/paths.cpp
> > Lines 497-498 (patched)
> > 
> >
> > I'll do:
> > ```
> > FAIL() << "Unsupported DiskInfo.Source.type";
> > ```

`FAIL` comes from gtest which is currently not available when compiling 
non-test code (e.g., not in the include path). Should I add it?

I went with `CHECK_NE` here as I saw it used for similar hard checks in 
non-test code. It is provided by glog which we use here.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57911/#review170321
---


On March 24, 2017, 4:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57911/
> ---
> 
> (Updated March 24, 2017, 4:39 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduce an explicit UNKNOWN enum kind to allow explicit handling
> of unknown enum values (e.g., when the sending and receiving end use
> different versions of a message using the enum).
> 
> This commit also migrates pattern matching of values of this enum from
> if statements to switch statements so that compiler diagnostics can be
> used to identify unhandled cases when other types are added in the
> future.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a66967ab459f75f21faf21be644f39b3fad670b 
>   include/mesos/v1/mesos.proto 115f1b4d9e129f83a3aed62a95eb11faa12e04d1 
>   src/common/resources.cpp ca1add1dbbe04fa775004010ebc847254c198bd7 
>   src/slave/paths.cpp 38ad1993aa36a627ec97a7865488677495ee4c5a 
>   src/slave/slave.cpp c6ee4fa9ad216fe82e97095e1808bb1fb1585398 
>   src/v1/resources.cpp 6008a9d2572d04612f0da2d54834be548d366f29 
> 
> 
> Diff: https://reviews.apache.org/r/57911/diff/1/
> 
> 
> Testing
> ---
> 
> make check (OS X, Linux)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 57911: WIP: Added UNKNOWN DiskInfo.Source type.

[Jie Yu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57911/#review170321
---


Fix it, then Ship it!





src/slave/paths.cpp
Lines 497-498 (patched)


I'll do:
```
FAIL() << "Unsupported DiskInfo.Source.type";
```


- Jie Yu


On March 24, 2017, 3:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57911/
> ---
> 
> (Updated March 24, 2017, 3:39 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduce an explicit UNKNOWN enum kind to allow explicit handling
> of unknown enum values (e.g., when the sending and receiving end use
> different versions of a message using the enum).
> 
> This commit also migrates pattern matching of values of this enum from
> if statements to switch statements so that compiler diagnostics can be
> used to identify unhandled cases when other types are added in the
> future.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a66967ab459f75f21faf21be644f39b3fad670b 
>   include/mesos/v1/mesos.proto 115f1b4d9e129f83a3aed62a95eb11faa12e04d1 
>   src/common/resources.cpp ca1add1dbbe04fa775004010ebc847254c198bd7 
>   src/slave/paths.cpp 38ad1993aa36a627ec97a7865488677495ee4c5a 
>   src/slave/slave.cpp c6ee4fa9ad216fe82e97095e1808bb1fb1585398 
>   src/v1/resources.cpp 6008a9d2572d04612f0da2d54834be548d366f29 
> 
> 
> Diff: https://reviews.apache.org/r/57911/diff/1/
> 
> 
> Testing
> ---
> 
> make check (OS X, Linux)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 57911: WIP: Added UNKNOWN DiskInfo.Source type.

[Mesos Reviewbot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57911/#review170029
---



Patch looks great!

Reviews applied: [57911]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On March 24, 2017, 3:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57911/
> ---
> 
> (Updated March 24, 2017, 3:39 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We introduce an explicit UNKNOWN enum kind to allow explicit handling
> of unknown enum values (e.g., when the sending and receiving end use
> different versions of a message using the enum).
> 
> This commit also migrates pattern matching of values of this enum from
> if statements to switch statements so that compiler diagnostics can be
> used to identify unhandled cases when other types are added in the
> future.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a66967ab459f75f21faf21be644f39b3fad670b 
>   include/mesos/v1/mesos.proto 115f1b4d9e129f83a3aed62a95eb11faa12e04d1 
>   src/common/resources.cpp ca1add1dbbe04fa775004010ebc847254c198bd7 
>   src/slave/paths.cpp 38ad1993aa36a627ec97a7865488677495ee4c5a 
>   src/slave/slave.cpp c6ee4fa9ad216fe82e97095e1808bb1fb1585398 
>   src/v1/resources.cpp 6008a9d2572d04612f0da2d54834be548d366f29 
> 
> 
> Diff: https://reviews.apache.org/r/57911/diff/1/
> 
> 
> Testing
> ---
> 
> make check (OS X, Linux)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 57911: WIP: Added UNKNOWN DiskInfo.Source type.

[Benjamin Bannier]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57911/
---

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

We introduce an explicit UNKNOWN enum kind to allow explicit handling
of unknown enum values (e.g., when the sending and receiving end use
different versions of a message using the enum).

This commit also migrates pattern matching of values of this enum from
if statements to switch statements so that compiler diagnostics can be
used to identify unhandled cases when other types are added in the
future.


Diffs
-

  include/mesos/mesos.proto 9a66967ab459f75f21faf21be644f39b3fad670b 
  include/mesos/v1/mesos.proto 115f1b4d9e129f83a3aed62a95eb11faa12e04d1 
  src/common/resources.cpp ca1add1dbbe04fa775004010ebc847254c198bd7 
  src/slave/paths.cpp 38ad1993aa36a627ec97a7865488677495ee4c5a 
  src/slave/slave.cpp c6ee4fa9ad216fe82e97095e1808bb1fb1585398 
  src/v1/resources.cpp 6008a9d2572d04612f0da2d54834be548d366f29 


Diff: https://reviews.apache.org/r/57911/diff/1/


Testing
---

make check (OS X, Linux)


Thanks,

Benjamin Bannier