You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by "Timothy St. Clair" <ts...@redhat.com> on 2014/01/20 20:43:54 UTC

Review Request 17130: Removal of --without-curl

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

Review request for mesos and Timothy St. Clair.


Repository: mesos-git


Description
-------

removal of --without-curl 


Diffs
-----

  configure.ac 9f96b7d0790715530b1c09f9991d68a921afc68c 

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


Testing
-------


Thanks,

Timothy St. Clair


Re: Review Request 17130: Removal of --without-curl

Posted by "Timothy St. Clair" <ts...@redhat.com>.

> On Jan. 21, 2014, 6:34 p.m., Ben Mahler wrote:
> > configure.ac, line 599
> > <https://reviews.apache.org/r/17130/diff/1/?file=432899#file432899line599>
> >
> >     Can you kill the whitespace here and below?

Can't we add a pre-commit hook script to fix a lot of style questions and automate this?  


- Timothy


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


On Jan. 22, 2014, 8 p.m., Timothy St. Clair wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17130/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 8 p.m.)
> 
> 
> Review request for mesos and Timothy St. Clair.
> 
> 
> Bugs: MESOS-925
>     https://issues.apache.org/jira/browse/MESOS-925
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> removal of --without-curl 
> 
> 
> Diffs
> -----
> 
>   configure.ac 9f96b7d 
> 
> Diff: https://reviews.apache.org/r/17130/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Timothy St. Clair
> 
>


Re: Review Request 17130: Removal of --without-curl

Posted by Ben Mahler <be...@gmail.com>.

> On Jan. 21, 2014, 6:34 p.m., Ben Mahler wrote:
> > configure.ac, line 599
> > <https://reviews.apache.org/r/17130/diff/1/?file=432899#file432899line599>
> >
> >     Can you kill the whitespace here and below?
> 
> Timothy St. Clair wrote:
>     Can't we add a pre-commit hook script to fix a lot of style questions and automate this?

Seems good to me. In the past we had thought about putting these kinds of checks into post-reviews.py to catch things before the review is published.


- Ben


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


On Jan. 22, 2014, 8 p.m., Timothy St. Clair wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17130/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 8 p.m.)
> 
> 
> Review request for mesos and Timothy St. Clair.
> 
> 
> Bugs: MESOS-925
>     https://issues.apache.org/jira/browse/MESOS-925
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> removal of --without-curl 
> 
> 
> Diffs
> -----
> 
>   configure.ac 9f96b7d 
> 
> Diff: https://reviews.apache.org/r/17130/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Timothy St. Clair
> 
>


Re: Review Request 17130: Removal of --without-curl

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17130/#review32388
-----------------------------------------------------------

Ship it!


Just some style nits below.


configure.ac
<https://reviews.apache.org/r/17130/#comment61178>

    Can you kill the whitespace here and below?



configure.ac
<https://reviews.apache.org/r/17130/#comment61177>

    s/curl/libcurl/
    s/build/build./


- Ben Mahler


On Jan. 21, 2014, 6:32 p.m., Timothy St. Clair wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17130/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2014, 6:32 p.m.)
> 
> 
> Review request for mesos and Timothy St. Clair.
> 
> 
> Bugs: MESOS-925
>     https://issues.apache.org/jira/browse/MESOS-925
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> removal of --without-curl 
> 
> 
> Diffs
> -----
> 
>   configure.ac 9f96b7d0790715530b1c09f9991d68a921afc68c 
> 
> Diff: https://reviews.apache.org/r/17130/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Timothy St. Clair
> 
>


Re: Review Request 17130: Removal of --without-curl

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17130/#review32401
-----------------------------------------------------------



configure.ac
<https://reviews.apache.org/r/17130/#comment61189>

    It would be nice to be more descriptive in our messages here as to why we need libcrypto for libcurl, is your TODO to determine whether we really need to check for libcrypto and libssl?
    
    IIRC, libcurl can be built with or without requiring libcrypto / libssl.


- Ben Mahler


On Jan. 21, 2014, 6:32 p.m., Timothy St. Clair wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17130/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2014, 6:32 p.m.)
> 
> 
> Review request for mesos and Timothy St. Clair.
> 
> 
> Bugs: MESOS-925
>     https://issues.apache.org/jira/browse/MESOS-925
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> removal of --without-curl 
> 
> 
> Diffs
> -----
> 
>   configure.ac 9f96b7d0790715530b1c09f9991d68a921afc68c 
> 
> Diff: https://reviews.apache.org/r/17130/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Timothy St. Clair
> 
>


Re: Review Request 17130: Removal of --without-curl

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17130/#review32578
-----------------------------------------------------------

Ship it!


Thanks Tim! Submitted.

- Ben Mahler


On Jan. 22, 2014, 8 p.m., Timothy St. Clair wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17130/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 8 p.m.)
> 
> 
> Review request for mesos and Timothy St. Clair.
> 
> 
> Bugs: MESOS-925
>     https://issues.apache.org/jira/browse/MESOS-925
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> removal of --without-curl 
> 
> 
> Diffs
> -----
> 
>   configure.ac 9f96b7d 
> 
> Diff: https://reviews.apache.org/r/17130/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Timothy St. Clair
> 
>


Re: Review Request 17130: Removal of --without-curl

Posted by "Timothy St. Clair" <ts...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17130/
-----------------------------------------------------------

(Updated Jan. 22, 2014, 8 p.m.)


Review request for mesos and Timothy St. Clair.


Changes
-------

Update to remove ssl and crypto checks.  Transitive dependency graph, is a safe assumption.
Also update libz comment.


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


Repository: mesos-git


Description
-------

removal of --without-curl 


Diffs (updated)
-----

  configure.ac 9f96b7d 

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


Testing
-------


Thanks,

Timothy St. Clair


Re: Review Request 17130: Removal of --without-curl

Posted by Benjamin Mahler <be...@gmail.com>.
Sounds good


On Wed, Jan 22, 2014 at 11:45 AM, Timothy St. Clair <ts...@redhat.com>wrote:

>
>
> > On Jan. 22, 2014, 7:41 p.m., Ben Mahler wrote:
> > > Hey Tim, this looks good. I'm just wondering about the checks for
> libcrypto and libssl, do we need to check for these?
> > >
> > > Wouldn't checking for libcurl be sufficient if we can assume libcurl's
> dependencies are present?
>
> Agreed.  Apparently my last comment response was not published.  Why don't
> I remove and update this patch once more.
>
>
> - Timothy
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17130/#review32522
> -----------------------------------------------------------
>
>
> On Jan. 22, 2014, 5:30 p.m., Timothy St. Clair wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/17130/
> > -----------------------------------------------------------
> >
> > (Updated Jan. 22, 2014, 5:30 p.m.)
> >
> >
> > Review request for mesos and Timothy St. Clair.
> >
> >
> > Bugs: MESOS-925
> >     https://issues.apache.org/jira/browse/MESOS-925
> >
> >
> > Repository: mesos-git
> >
> >
> > Description
> > -------
> >
> > removal of --without-curl
> >
> >
> > Diffs
> > -----
> >
> >   configure.ac 9f96b7d
> >
> > Diff: https://reviews.apache.org/r/17130/diff/
> >
> >
> > Testing
> > -------
> >
> >
> > Thanks,
> >
> > Timothy St. Clair
> >
> >
>
>

Re: Review Request 17130: Removal of --without-curl

Posted by "Timothy St. Clair" <ts...@redhat.com>.

> On Jan. 22, 2014, 7:41 p.m., Ben Mahler wrote:
> > Hey Tim, this looks good. I'm just wondering about the checks for libcrypto and libssl, do we need to check for these?
> > 
> > Wouldn't checking for libcurl be sufficient if we can assume libcurl's dependencies are present?

Agreed.  Apparently my last comment response was not published.  Why don't I remove and update this patch once more. 


- Timothy


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


On Jan. 22, 2014, 5:30 p.m., Timothy St. Clair wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17130/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 5:30 p.m.)
> 
> 
> Review request for mesos and Timothy St. Clair.
> 
> 
> Bugs: MESOS-925
>     https://issues.apache.org/jira/browse/MESOS-925
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> removal of --without-curl 
> 
> 
> Diffs
> -----
> 
>   configure.ac 9f96b7d 
> 
> Diff: https://reviews.apache.org/r/17130/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Timothy St. Clair
> 
>


Re: Review Request 17130: Removal of --without-curl

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17130/#review32522
-----------------------------------------------------------


Hey Tim, this looks good. I'm just wondering about the checks for libcrypto and libssl, do we need to check for these?

Wouldn't checking for libcurl be sufficient if we can assume libcurl's dependencies are present?


configure.ac
<https://reviews.apache.org/r/17130/#comment61389>

    zlib is needed regardless of libcurl (see gzip.hpp in stout that is included in libprocess), can you update this error message to reflect why we need zlib?


- Ben Mahler


On Jan. 22, 2014, 5:30 p.m., Timothy St. Clair wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17130/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2014, 5:30 p.m.)
> 
> 
> Review request for mesos and Timothy St. Clair.
> 
> 
> Bugs: MESOS-925
>     https://issues.apache.org/jira/browse/MESOS-925
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> removal of --without-curl 
> 
> 
> Diffs
> -----
> 
>   configure.ac 9f96b7d 
> 
> Diff: https://reviews.apache.org/r/17130/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Timothy St. Clair
> 
>


Re: Review Request 17130: Removal of --without-curl

Posted by "Timothy St. Clair" <ts...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17130/
-----------------------------------------------------------

(Updated Jan. 22, 2014, 5:30 p.m.)


Review request for mesos and Timothy St. Clair.


Changes
-------

Updated patch per-review


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


Repository: mesos-git


Description
-------

removal of --without-curl 


Diffs (updated)
-----

  configure.ac 9f96b7d 

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


Testing
-------


Thanks,

Timothy St. Clair


Re: Review Request 17130: Removal of --without-curl

Posted by "Timothy St. Clair" <ts...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17130/
-----------------------------------------------------------

(Updated Jan. 21, 2014, 6:32 p.m.)


Review request for mesos and Timothy St. Clair.


Changes
-------

Added bug.


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


Repository: mesos-git


Description
-------

removal of --without-curl 


Diffs
-----

  configure.ac 9f96b7d0790715530b1c09f9991d68a921afc68c 

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


Testing
-------


Thanks,

Timothy St. Clair