Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

(Updated Feb. 9, 2016, 1:49 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Review comment


Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs (updated)
-

  src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8 

Diff: https://reviews.apache.org/r/41288/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

> On Feb. 9, 2016, 1:10 a.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, line 948
> > 
> >
> > looks like you forgot to pass "this" to this callback?

Good catch. Fixed


- Anand


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


On Feb. 9, 2016, 1:49 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> ---
> 
> (Updated Feb. 9, 2016, 1:49 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces a versioned callback interface for testing HTTP based 
> executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
> for Schedulers.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8 
> 
> Diff: https://reviews.apache.org/r/41288/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

(Updated Feb. 8, 2016, 10:15 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Review comments from Vinod


Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs (updated)
-

  src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8 

Diff: https://reviews.apache.org/r/41288/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

> On Feb. 8, 2016, 10 p.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, line 960
> > 
> >
> > s/executor_/executor/
> > 
> > we typically don't use underscores for member variables.

Our style guide does recommend to prefer trailing underscores for member 
variables. From the style guide:

```
Prefer trailing underscores for use as member fields (but not required).
```

I removed the trailing underscores for now since all the other classes/structs 
in this file do not use it.


> On Feb. 8, 2016, 10 p.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, line 965
> > 
> >
> > do you need executor::TestMesos even though you are in executor 
> > namespace?

Yep, right. My bad, fixed. :-)


- Anand


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


On Feb. 8, 2016, 10:15 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> ---
> 
> (Updated Feb. 8, 2016, 10:15 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces a versioned callback interface for testing HTTP based 
> executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
> for Schedulers.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8 
> 
> Diff: https://reviews.apache.org/r/41288/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Vinod Kone]

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




src/tests/mesos.hpp (line 937)


s/executor/_executor/



src/tests/mesos.hpp (line 943)


put the second argument on next line for consistency.

also pass "this" as the 3rd argument for consistency with connected 
callback.



src/tests/mesos.hpp (line 960)


s/executor_/executor/

we typically don't use underscores for member variables.



src/tests/mesos.hpp (line 965)


do you need executor::TestMesos even though you are in executor namespace?


- Vinod Kone


On Feb. 4, 2016, 11:11 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> ---
> 
> (Updated Feb. 4, 2016, 11:11 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces a versioned callback interface for testing HTTP based 
> executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
> for Schedulers.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp c2bae4767ee7372c796bfad44ed1e86db7dd3488 
> 
> Diff: https://reviews.apache.org/r/41288/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

(Updated Feb. 9, 2016, 12:12 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Review comments


Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs (updated)
-

  src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8 

Diff: https://reviews.apache.org/r/41288/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Vinod Kone]

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


Fix it, then Ship it!





src/tests/mesos.hpp (line 948)


looks like you forgot to pass "this" to this callback?


- Vinod Kone


On Feb. 9, 2016, 12:12 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> ---
> 
> (Updated Feb. 9, 2016, 12:12 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces a versioned callback interface for testing HTTP based 
> executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
> for Schedulers.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e07d8aa6f1e507a91ce70763aafa134bdd9a7ec8 
> 
> Diff: https://reviews.apache.org/r/41288/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

(Updated Feb. 4, 2016, 11:11 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Review comments


Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs (updated)
-

  src/tests/mesos.hpp c2bae4767ee7372c796bfad44ed1e86db7dd3488 

Diff: https://reviews.apache.org/r/41288/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

(Updated Feb. 3, 2016, 10:14 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Updated deps since the previous change has now been committed. Would address 
review comments in next iteration.


Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 

Diff: https://reviews.apache.org/r/41288/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Vinod Kone]

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




src/tests/mesos.hpp (line 839)


The naming here is a bit confusing.

How about putting this class and the above in an executor namespace and do 
something like this:

```
namespace mesos {
namespace internal {
namespace tests {
...
...
...
namespace executor {

template
class Callbacks
{

};

template 
class TestMesos:: public Mesos
{

};

}
...
...
...
}
}
}

```

One issue might be that someone might be confused between the top level 
MesosTest class and TestMesos class, but I think the namespace should alleviate 
 the confusion.



src/tests/mesos.hpp (line 850)


each argument should be on its own line when wrapping. put `lambda::_1` on 
the next line.



src/tests/mesos.hpp (line 853)


why is this public?



src/tests/mesos.hpp (line 863)


should this be an owned pointer?



src/tests/mesos.hpp (line 869)


new line.


- Vinod Kone


On Jan. 19, 2016, 10:28 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41288/
> ---
> 
> (Updated Jan. 19, 2016, 10:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-4433
> https://issues.apache.org/jira/browse/MESOS-4433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces a versioned callback interface for testing HTTP based 
> executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
> for Schedulers.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 
> 
> Diff: https://reviews.apache.org/r/41288/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 10:28 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Update deps


Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 

Diff: https://reviews.apache.org/r/41288/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

(Updated Jan. 19, 2016, 8:05 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Updated Bugs link


Bugs: MESOS-4433
https://issues.apache.org/jira/browse/MESOS-4433


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 

Diff: https://reviews.apache.org/r/41288/diff/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 41288: Introduced an callback interface for testing HTTP based executors.

[Anand Mazumdar]

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

(Updated Jan. 12, 2016, 9:21 a.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
---

Rebased


Bugs: MESOS-3550
https://issues.apache.org/jira/browse/MESOS-3550


Repository: mesos


Description
---

This change introduces a versioned callback interface for testing HTTP based 
executors. The reasoning is similar to `MESOS-3339` , the corresponding issue 
for Schedulers.


Diffs (updated)
-

  src/tests/mesos.hpp 3d9ebc6c9dc3cd1be02dc3771fbd847386907fac 

Diff: https://reviews.apache.org/r/41288/diff/


Testing
---

make check


Thanks,

Anand Mazumdar