Re: Review Request 36360: Adding common constants for HTTP API

2015-07-15 Thread Ben Mahler 

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

Ship it!



src/common/http.hpp 


Whoops, this is still used?



src/common/http.cpp (line 44)


Whoops, space after =?


- Ben Mahler


On July 15, 2015, 2:50 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 15, 2015, 2:50 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
> https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding constants used commonly through the different HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp bbd063d 
>   src/common/http.cpp 73a4de1 
> 
> Diff: https://reviews.apache.org/r/36360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-14 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36360]

All tests passed.

- Mesos ReviewBot


On July 15, 2015, 2:50 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 15, 2015, 2:50 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
> https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding constants used commonly through the different HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp bbd063d 
>   src/common/http.cpp 73a4de1 
> 
> Diff: https://reviews.apache.org/r/36360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-14 Thread Isabel Jimenez 

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

(Updated July 15, 2015, 2:50 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
Massenzio, and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Adding constants used commonly through the different HTTP endpoints


Diffs (updated)
-

  src/common/http.hpp bbd063d 
  src/common/http.cpp 73a4de1 

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


Testing
---


Thanks,

Isabel Jimenez

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-10 Thread Alexander Rukletsov 

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



src/common/http.hpp (lines 52 - 53)


Let's convert them to static functions as per 
https://issues.apache.org/jira/browse/MESOS-1023. E.g. see 
https://reviews.apache.org/r/30841/.


- Alexander Rukletsov


On July 9, 2015, 10:34 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 9, 2015, 10:34 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
> https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding constants used commonly through the different HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp bbd063d 
>   src/common/http.cpp 73a4de1 
> 
> Diff: https://reviews.apache.org/r/36360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Marco Massenzio 

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

Ship it!


Ship It!

- Marco Massenzio


On July 9, 2015, 10:34 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 9, 2015, 10:34 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
> https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding constants used commonly through the different HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp bbd063d 
>   src/common/http.cpp 73a4de1 
> 
> Diff: https://reviews.apache.org/r/36360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36360]

All tests passed.

- Mesos ReviewBot


On July 9, 2015, 10:34 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 9, 2015, 10:34 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
> https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding constants used commonly through the different HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp bbd063d 
>   src/common/http.cpp 73a4de1 
> 
> Diff: https://reviews.apache.org/r/36360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Marco Massenzio 


> On July 9, 2015, 9:32 p.m., Ben Mahler wrote:
> > Can you move this into the existing common/http.hpp, and remove the content 
> > type one? For content type, would rather see a typed member on 
> > Request/Response than constants here, given the other occurrences:
> > 
> > ```
> > ?  mesos git:(master) ? grep -R Content-Type src | grep -v js | grep -v html
> > src/files/files.cpp:  response.headers["Content-Type"] = 
> > "application/octet-stream";
> > src/files/files.cpp:  response.headers["Content-Type"] = 
> > mime::types[extension];
> > src/tests/fault_tolerance_tests.cpp:  "Content-Type",
> > src/tests/files_tests.cpp:  "Content-Type",
> > src/tests/files_tests.cpp:  AWAIT_EXPECT_RESPONSE_HEADER_EQ("image/gif", 
> > "Content-Type", response);
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/metrics_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/metrics_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/monitor_tests.cpp:  "Content-Type",
> > src/tests/monitor_tests.cpp:  "Content-Type",
> > src/tests/monitor_tests.cpp:  "Content-Type",
> > src/tests/monitor_tests.cpp:  "Content-Type",
> > src/tests/repair_tests.cpp:"Content-Type",  
> >   \
> > src/tests/scheduler_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/slave_tests.cpp:  response.get().headers.get("Content-Type"));
> > src/tests/slave_tests.cpp:  response.get().headers.get("Content-Type"));
> > src/tests/slave_tests.cpp:  response.get().headers.get("Content-Type"));
> > src/tests/utils.cpp:  response.get().headers.get("Content-Type"));
> > ?  mesos git:(master) ? grep -R Content-Type 3rdparty | grep -v js | grep 
> > -v html
> > 3rdparty/libprocess/examples/example.cpp:
> > response.headers["Content-Type"] = "text/plain";
> > 3rdparty/libprocess/examples/example.cpp:
> > response.headers["Content-Type"] = "text/plain";
> > 3rdparty/libprocess/include/process/http.hpp:  // specify the 
> > 'Content-Type' header, but the 'Content-Length' and
> > 3rdparty/libprocess/include/process/http.hpp:  headers["Content-Type"] 
> > = "text/javascript";
> > 3rdparty/libprocess/include/process/process.hpp:  // '/path/file'. The 
> > 'Content-Type' header of the HTTP response will
> > 3rdparty/libprocess/src/help.cpp:response.headers["Content-Type"] = 
> > "text/x-markdown";
> > 3rdparty/libprocess/src/http.cpp:  // Overwrite Content-Type if necessary.
> > 3rdparty/libprocess/src/http.cpp:headers["Content-Type"] = 
> > contentType.get();
> > 3rdparty/libprocess/src/http.cpp:return Failure("Attempted to do a POST 
> > with a Content-Type but no body");
> > 3rdparty/libprocess/src/http.cpp:return Failure("Attempted to do a POST 
> > with a Content-Type but no body");
> > 3rdparty/libprocess/src/process.cpp:// While the user is expected 
> > to properly set a 'Content-Type'
> > 3rdparty/libprocess/src/process.cpp:// While the user is expected to 
> > properly set a 'Content-Type'
> > 3rdparty/libprocess/src/process.cpp:// Try and determine the 
> > Content-Type from an extension.
> > 3rdparty/libprocess/src/process.cpp:
> > response.headers["Content-Type"] = assets[name].types[extension];
> > 3rdparty/libprocess/src/profiler.cpp:  response.headers["Content-Type"] = 
> > "application/octet-stream";
> > 3rdparty/libprocess/src/tests/decoder_tests.cpp:"Content-Type: 
> > text/plain\r\n"
> > 3rdparty/libprocess/src/tests/decoder_tests.cpp:"Content-Type: 
> > text/plain\r\n"
> > 3rdparty/libprocess/src/tests/decoder_tests.cpp:"Content-Type: 
> > text/plain\r\n"
> > 3rdparty/libprocess/src/tests/http_tests.cpp:  headers["Content-Type"] = 
> > "text/plain";
> > 3rdparty/libprocess/src/tests/http_tests.cpp:  EXPECT_EQ("text/javascript", 
> > response.headers["Content-Type"]);
> > 3rdparty/libprocess/src/tests/system_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > ```
> 
> Isabel Jimenez wrote:
> ok :)

LOL - there's something to be said about hard-coded strings :)
Thanks, Ben!


- Marco


---
Th

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Isabel Jimenez 


> On July 9, 2015, 9:32 p.m., Ben Mahler wrote:
> > Can you move this into the existing common/http.hpp, and remove the content 
> > type one? For content type, would rather see a typed member on 
> > Request/Response than constants here, given the other occurrences:
> > 
> > ```
> > ?  mesos git:(master) ? grep -R Content-Type src | grep -v js | grep -v html
> > src/files/files.cpp:  response.headers["Content-Type"] = 
> > "application/octet-stream";
> > src/files/files.cpp:  response.headers["Content-Type"] = 
> > mime::types[extension];
> > src/tests/fault_tolerance_tests.cpp:  "Content-Type",
> > src/tests/files_tests.cpp:  "Content-Type",
> > src/tests/files_tests.cpp:  AWAIT_EXPECT_RESPONSE_HEADER_EQ("image/gif", 
> > "Content-Type", response);
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/master_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/metrics_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/metrics_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/monitor_tests.cpp:  "Content-Type",
> > src/tests/monitor_tests.cpp:  "Content-Type",
> > src/tests/monitor_tests.cpp:  "Content-Type",
> > src/tests/monitor_tests.cpp:  "Content-Type",
> > src/tests/repair_tests.cpp:"Content-Type",  
> >   \
> > src/tests/scheduler_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > src/tests/slave_tests.cpp:  response.get().headers.get("Content-Type"));
> > src/tests/slave_tests.cpp:  response.get().headers.get("Content-Type"));
> > src/tests/slave_tests.cpp:  response.get().headers.get("Content-Type"));
> > src/tests/utils.cpp:  response.get().headers.get("Content-Type"));
> > ?  mesos git:(master) ? grep -R Content-Type 3rdparty | grep -v js | grep 
> > -v html
> > 3rdparty/libprocess/examples/example.cpp:
> > response.headers["Content-Type"] = "text/plain";
> > 3rdparty/libprocess/examples/example.cpp:
> > response.headers["Content-Type"] = "text/plain";
> > 3rdparty/libprocess/include/process/http.hpp:  // specify the 
> > 'Content-Type' header, but the 'Content-Length' and
> > 3rdparty/libprocess/include/process/http.hpp:  headers["Content-Type"] 
> > = "text/javascript";
> > 3rdparty/libprocess/include/process/process.hpp:  // '/path/file'. The 
> > 'Content-Type' header of the HTTP response will
> > 3rdparty/libprocess/src/help.cpp:response.headers["Content-Type"] = 
> > "text/x-markdown";
> > 3rdparty/libprocess/src/http.cpp:  // Overwrite Content-Type if necessary.
> > 3rdparty/libprocess/src/http.cpp:headers["Content-Type"] = 
> > contentType.get();
> > 3rdparty/libprocess/src/http.cpp:return Failure("Attempted to do a POST 
> > with a Content-Type but no body");
> > 3rdparty/libprocess/src/http.cpp:return Failure("Attempted to do a POST 
> > with a Content-Type but no body");
> > 3rdparty/libprocess/src/process.cpp:// While the user is expected 
> > to properly set a 'Content-Type'
> > 3rdparty/libprocess/src/process.cpp:// While the user is expected to 
> > properly set a 'Content-Type'
> > 3rdparty/libprocess/src/process.cpp:// Try and determine the 
> > Content-Type from an extension.
> > 3rdparty/libprocess/src/process.cpp:
> > response.headers["Content-Type"] = assets[name].types[extension];
> > 3rdparty/libprocess/src/profiler.cpp:  response.headers["Content-Type"] = 
> > "application/octet-stream";
> > 3rdparty/libprocess/src/tests/decoder_tests.cpp:"Content-Type: 
> > text/plain\r\n"
> > 3rdparty/libprocess/src/tests/decoder_tests.cpp:"Content-Type: 
> > text/plain\r\n"
> > 3rdparty/libprocess/src/tests/decoder_tests.cpp:"Content-Type: 
> > text/plain\r\n"
> > 3rdparty/libprocess/src/tests/http_tests.cpp:  headers["Content-Type"] = 
> > "text/plain";
> > 3rdparty/libprocess/src/tests/http_tests.cpp:  EXPECT_EQ("text/javascript", 
> > response.headers["Content-Type"]);
> > 3rdparty/libprocess/src/tests/system_tests.cpp:  
> > response.get().headers.get("Content-Type"));
> > ```

ok :)


- Isabel


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

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Isabel Jimenez 

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

(Updated July 9, 2015, 10:34 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
Massenzio, and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Adding constants used commonly through the different HTTP endpoints


Diffs (updated)
-

  src/common/http.hpp bbd063d 
  src/common/http.cpp 73a4de1 

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


Testing
---


Thanks,

Isabel Jimenez

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Ben Mahler 

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


Can you move this into the existing common/http.hpp, and remove the content 
type one? For content type, would rather see a typed member on Request/Response 
than constants here, given the other occurrences:

```
➜  mesos git:(master) ✗ grep -R Content-Type src | grep -v js | grep -v html
src/files/files.cpp:  response.headers["Content-Type"] = 
"application/octet-stream";
src/files/files.cpp:  response.headers["Content-Type"] = 
mime::types[extension];
src/tests/fault_tolerance_tests.cpp:  "Content-Type",
src/tests/files_tests.cpp:  "Content-Type",
src/tests/files_tests.cpp:  AWAIT_EXPECT_RESPONSE_HEADER_EQ("image/gif", 
"Content-Type", response);
src/tests/master_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/master_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/master_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/master_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/master_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/master_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/master_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/master_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/master_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/metrics_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/metrics_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/monitor_tests.cpp:  "Content-Type",
src/tests/monitor_tests.cpp:  "Content-Type",
src/tests/monitor_tests.cpp:  "Content-Type",
src/tests/monitor_tests.cpp:  "Content-Type",
src/tests/repair_tests.cpp:"Content-Type",  
  \
src/tests/scheduler_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/slave_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/slave_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/slave_tests.cpp:  response.get().headers.get("Content-Type"));
src/tests/utils.cpp:  response.get().headers.get("Content-Type"));
➜  mesos git:(master) ✗ grep -R Content-Type 3rdparty | grep -v js | grep -v 
html
3rdparty/libprocess/examples/example.cpp:response.headers["Content-Type"] = 
"text/plain";
3rdparty/libprocess/examples/example.cpp:
response.headers["Content-Type"] = "text/plain";
3rdparty/libprocess/include/process/http.hpp:  // specify the 'Content-Type' 
header, but the 'Content-Length' and
3rdparty/libprocess/include/process/http.hpp:  headers["Content-Type"] = 
"text/javascript";
3rdparty/libprocess/include/process/process.hpp:  // '/path/file'. The 
'Content-Type' header of the HTTP response will
3rdparty/libprocess/src/help.cpp:response.headers["Content-Type"] = 
"text/x-markdown";
3rdparty/libprocess/src/http.cpp:  // Overwrite Content-Type if necessary.
3rdparty/libprocess/src/http.cpp:headers["Content-Type"] = 
contentType.get();
3rdparty/libprocess/src/http.cpp:return Failure("Attempted to do a POST 
with a Content-Type but no body");
3rdparty/libprocess/src/http.cpp:return Failure("Attempted to do a POST 
with a Content-Type but no body");
3rdparty/libprocess/src/process.cpp:// While the user is expected to 
properly set a 'Content-Type'
3rdparty/libprocess/src/process.cpp:// While the user is expected to 
properly set a 'Content-Type'
3rdparty/libprocess/src/process.cpp:// Try and determine the Content-Type 
from an extension.
3rdparty/libprocess/src/process.cpp:response.headers["Content-Type"] = 
assets[name].types[extension];
3rdparty/libprocess/src/profiler.cpp:  response.headers["Content-Type"] = 
"application/octet-stream";
3rdparty/libprocess/src/tests/decoder_tests.cpp:"Content-Type: 
text/plain\r\n"
3rdparty/libprocess/src/tests/decoder_tests.cpp:"Content-Type: 
text/plain\r\n"
3rdparty/libprocess/src/tests/decoder_tests.cpp:"Content-Type: 
text/plain\r\n"
3rdparty/libprocess/src/tests/http_tests.cpp:  headers["Content-Type"] = 
"text/plain";
3rdparty/libprocess/src/tests/http_tests.cpp:  EXPECT_EQ("text/javascript", 
response.headers["Content-Type"]);
3rdparty/libprocess/src/tests/system_tests.cpp:  
response.get().headers.get("Content-Type"));
```

- Ben Mahler


On July 9, 2015, 9:07 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 9, 2015, 9:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjami

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [36360]

All tests passed.

- Mesos ReviewBot


On July 9, 2015, 9:07 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 9, 2015, 9:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
> https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding constants used commonly through the different HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e5b5d36 
>   src/common/http_constants.hpp PRE-CREATION 
>   src/common/http_constants.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Isabel Jimenez 


> On July 9, 2015, 8:23 p.m., Anand Mazumdar wrote:
> > src/common/http_constants.cpp, line 26
> > 
> >
> > minor nit-pick , might consider using std::string; before-hand ?

:) done


- Isabel


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


On July 9, 2015, 9:07 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 9, 2015, 9:07 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
> https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding constants used commonly through the different HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e5b5d36 
>   src/common/http_constants.hpp PRE-CREATION 
>   src/common/http_constants.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Isabel Jimenez 

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

(Updated July 9, 2015, 9:07 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
Massenzio, and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Adding constants used commonly through the different HTTP endpoints


Diffs (updated)
-

  src/Makefile.am e5b5d36 
  src/common/http_constants.hpp PRE-CREATION 
  src/common/http_constants.cpp PRE-CREATION 

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


Testing
---


Thanks,

Isabel Jimenez

Re: Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Anand Mazumdar 

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

Ship it!


LGTM, Just some minor comments.


src/common/http_constants.hpp (line 18)


This is not a self-contained header file that we encourage everyone to 
build up, kindly do #include  and remove the adjacent include from the 
cpp file.



src/common/http_constants.hpp (line 26)


s/Supported Content-Tye and Accept headers/Supported media types for 
Content-Type and Accept headers.



src/common/http_constants.cpp (line 26)


minor nit-pick , might consider using std::string; before-hand ?


- Anand Mazumdar


On July 9, 2015, 7:58 p.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36360/
> ---
> 
> (Updated July 9, 2015, 7:58 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
> https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> ---
> 
> Adding constants used commonly through the different HTTP endpoints
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e5b5d36 
>   src/common/http_constants.hpp PRE-CREATION 
>   src/common/http_constants.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36360/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Review Request 36360: Adding common constants for HTTP API

2015-07-09 Thread Isabel Jimenez 

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

Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
Massenzio, and Vinod Kone.


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


Repository: mesos-incubating


Description
---

Adding constants used commonly through the different HTTP endpoints


Diffs
-

  src/Makefile.am e5b5d36 
  src/common/http_constants.hpp PRE-CREATION 
  src/common/http_constants.cpp PRE-CREATION 

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


Testing
---


Thanks,

Isabel Jimenez