This is an automatically generated e-mail. To reply, visit:

Looking great, just some minor thoughts around ways to make this a bit easier 
on the reader of the code. Curious to hear your thoughts!

support/generate-endpoint-help.py (line 35)

    typo: cause

support/generate-endpoint-help.py (line 36)

    typo: we

support/generate-endpoint-help.py (lines 55 - 60)

    Do we need the name indexing? It looks like we could just store the Popen 
objects in a list for cleanup purposes. Could start_master just return the 
Popen object instead of the index into this map?
    Since we only create the master followed by the agent, we don't have both 
subprocesses alive at the same time, could we simplify if we assume we won't 
have more than a single active subprocess for now?

support/generate-endpoint-help.py (lines 76 - 92)

    Would be nice to indent consistently here if possible, should we wrap the 
add_argument calls on a newline?

support/generate-endpoint-help.py (lines 77 - 79)

    Should this be a 4 space indent?

support/generate-endpoint-help.py (lines 100 - 111)

    Do we need these? They seem to exist only to rename os.makedirs and 
    For the os.makedirs error handling, we could just do the following in the 
caller code to make the 'already exists' case explicit:
    if not os.path.exists(directory):

support/generate-endpoint-help.py (line 103)

    Should this be a 2 space indent?

support/generate-endpoint-help.py (lines 114 - 127)

    Could we avoid the string -> list conversion by just starting with the 
command in list form?
    It looks like the caller code doesn't need to use a string.
    After that change, this could take s/cmd/args/

support/generate-endpoint-help.py (lines 133 - 134)

    Where does this happen? Looks like we don't delete the key from the map 
after the kill?

support/generate-endpoint-help.py (lines 186 - 188)

    A maintenance endpoint might be a good example to include in general, since 
these endpoints have a slash in the name?

support/generate-endpoint-help.py (lines 242 - 243)

    Could we prefix this sentence with a bold NOTE?

support/generate-endpoint-help.py (line 255)

    Let's omit any reference to mesosphere ;)

support/generate-endpoint-help.py (line 264)

    Ditto here ;)

support/generate-endpoint-help.py (lines 270 - 280)

    Still a little tricky for me to figure this out, could the comment include 
an example?
    One thought that comes to mind is, should this just return a 'list' of 
links for a given process, and the caller loops over processes calling this? 
Then the caller loops over the link list and stringifies as needed. Just 
thinking that perhaps this function should not be doing the overall string 
construction for the caller as that might be a bit tougher to follow.
    Curious to see if we can make this easier for dummies like me :)

support/generate-endpoint-help.py (line 304)


- Ben Mahler

On Feb. 5, 2016, 11:17 p.m., Kevin Klues wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43220/
> -----------------------------------------------------------
> (Updated Feb. 5, 2016, 11:17 p.m.)
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
> Bugs: MESOS-3831
>     https://issues.apache.org/jira/browse/MESOS-3831
> Repository: mesos
> Description
> -------
> Invoke this script from anywhere within the mesos source tree
> (including in the build directory) to autogenerate documentation for
> all endpoint strings and install them into the docs/endpoint folder.
> In a subsequent commit we commit all of these autogenerated files back
> to the source repo and add a link in home.md to find them.
> Diffs
> -----
>   support/generate-endpoint-help.py PRE-CREATION 
> Diff: https://reviews.apache.org/r/43220/diff/
> Testing
> -------
> Ran and generated files from it.  Then rebuilt the website and verified that 
> everything looked as it should.
> Thanks,
> Kevin Klues

Reply via email to