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


Would be great to add a unit test for VersionProcess as a follow up, not in 
this patch.


src/version/version.hpp (lines 40 - 41)
<https://reviews.apache.org/r/37024/#comment159135>

    Looks like this should be static?



src/version/version.cpp (lines 56 - 58)
<https://reviews.apache.org/r/37024/#comment159134>

    Couple of things:
    
    (1) Can you use a 2 space indent instead of 4?
    
    (2) Can you add the git fields here as well?
    
    ```
    "{",
    "  \"build_user\":\"username\",",
    "  \"build_time\":1443894750,",
    "  \"build_date\":\"2015-10-04 01:52:30\",",
    "  \"git_branch\":\"branch\", // Optional",
    "  \"git_sha\":\"2199a599d4e57cce0c9209660e488f530156e07b\", // Optional",
    "  \"git_tag\":\"0.26.0-rc1\", // Optional",
    "  \"version\":\"0.26.0\"",
    "}",
    ```
    
    (3) Ideally we could just make VersionProcess::version static and then just 
pretty print the json object directly here instead of hard coding an example. 
However, we don't currently have pretty printing, so feel free to just leave 
this example here and have a TODO for generating the example automatically.


- Ben Mahler


On Oct. 6, 2015, 2:02 a.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37024/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 2:02 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1841
>     https://issues.apache.org/jira/browse/MESOS-1841
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add an endpoint exposes Apache Mesos build informations and version 
> information.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am e69892736b0edc8c264eaccd52a04d44d01f53ba 
>   src/exec/exec.cpp 7b51baaa8c08d248918974a3a22b6217e388bcb1 
>   src/local/main.cpp 18b2f0187637cd425d55c220f73faac5a1218f0f 
>   src/master/main.cpp bafc605d6c20bd264b932e44ee80373a3f692734 
>   src/sched/sched.cpp 571e00d303009a940f17c8ed4582749a718e846d 
>   src/slave/main.cpp 364dc7fc7ab2e3cef01aea7267dafa014b60e2b9 
>   src/version/version.hpp PRE-CREATION 
>   src/version/version.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37024/diff/
> 
> 
> Testing
> -------
> 
> Manual test result:
> 
> ```
> $ curl http://localhost:5050/version 2>/dev/null|jq .
> 
> {
>   "version": "0.24.0",
>   "build_user": "haosdent",
>   "build_time": 1439702338,
>   "build_date": "2015-08-16 13:18:58"
> }
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Reply via email to