mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kapil Arya <ka...@mesosphere.io>
Subject Re: Review Request 53137: Fix Mesos build option argument handling.
Date Thu, 03 Nov 2016 14:21:55 GMT

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



Thanks for the patience, James! It mostly looks good but there are a few minor discrepencies.
Once you address them, we can commit it.


configure.ac (line 181)
<https://reviews.apache.org/r/53137/#comment224394>

    Also, if you don't mind, can you get rid of `default: no` from here and below?



configure.ac (line 224)
<https://reviews.apache.org/r/53137/#comment224393>

    Need to change `no` to `yes`



configure.ac (line 1825)
<https://reviews.apache.org/r/53137/#comment224386>

    Please change the test to:
        "x$enable_bundled" != "xyes"



configure.ac (line 2180)
<https://reviews.apache.org/r/53137/#comment224395>

    Maybe, also change this variable to `ENABLE_PYTHON_DEPENDENCY_INSTALL` and change the
test in src/Makefile.am to:
        if !ENABLE_PYTHON_DEPENDENCY_INSTALL
          ...
        fi
        
    This will improve readability, I guess.


- Kapil Arya


On Nov. 1, 2016, 12:24 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53137/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2016, 12:24 p.m.)
> 
> 
> Review request for mesos and Kapil Arya.
> 
> 
> Bugs: MESOS-2537
>     https://issues.apache.org/jira/browse/MESOS-2537
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fix all the AC_ARG_ENABLED options to correctly handle --enable-foo
> and --disable-foo variants. Update all the invocations to have a
> consistent calling convention.
> 
> 
> Diffs
> -----
> 
>   configure.ac c8d48be41d526c45ee00b404d0ad2b67ea11587f 
> 
> Diff: https://reviews.apache.org/r/53137/diff/
> 
> 
> Testing
> -------
> 
> Make check on OS X and Fedora 24.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message