> On March 26, 2016, 6:43 a.m., Jiang Yan Xu wrote:
> > configure.ac, lines 957-960
> > <https://reviews.apache.org/r/44945/diff/8/?file=1311200#file1311200line957>
> >
> >     Can we put AC_MSG_CHECKING before `AC_CHECK_HEADERS` and inside `AS_IF` 
> > above?
> >     
> >     i.e., AC_MSG_CHECKING/AC_MSG_CHECKING/AC_MSG_ERROR inform user what 
> > they don't already know (whether the system dependencies are met), not what 
> > they already know (the fact that they provided the 
> > `--enable_xfs_disk_isolator` flag).
> >     
> >     And this check is conditional: we only print `AC_MSG_CHECKING` and do 
> > these checks when `test "x$enable_xfs_disk_isolator" = "xyes"`; we print 
> > `AC_MSG_ERROR([...])` when such checks fail and `AC_MSG_RESULT([yes])` when 
> > all checks succeed.
> >     
> >     Would this work?
> >     
> >     If we do this, then the message could be `AC_MSG_CHECKING([whether we 
> > can enable the XFS disk isolator])`
> 
> James Peach wrote:
>     No, this message should be outside the check so that the builder always 
> knows what happened, independent of what configure flags they passed. 
> Typically, people start by not passing any flags, and it is helpful to 
> explicitly log that a feature was not enabled. Even if you pass the --enable 
> flags, it is comforting to get an explicit confirmation that the feature you 
> asked for is enabled.
> 
> Jiang Yan Xu wrote:
>     I agree that explicitly confirming that the user has enabled something is 
> better. 
>     
>     I still don't think we should print it after we've done all the 
> dependency checks for it and have potentially aborted configure with error 
> messages but without telling the error is due to "whether to enable the XFS 
> disk isolator..."
>     
>     i.e., the following is better.
>     
>     ```
>     checking whether to enable the XFS disk isolator... yes
>     
>     OR 
>     
>     checking whether to enable the XFS disk isolator... missing libblkid 
> headers
>     -------------------------------------------------------------------
>     Please install the libblkid development package.
>     -------------------------------------------------------------------
>     
>     OR
>     
>     checking whether to enable the XFS disk isolator... no / checking whether 
> to enable the XFS disk isolator... not enabled by user
>     
>     ```
>     
>     Agreed?

If you do it this way, you get the headers, etc check output intermingled in 
the middle of your nice clean feature check message.


- James


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


On March 26, 2016, 5:05 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44945/
> -----------------------------------------------------------
> 
> (Updated March 26, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
>     https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add autoconf tests for XFS project quotas.
> 
> 
> Diffs
> -----
> 
>   configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1fbbbb2d19ffb8 
> 
> Diff: https://reviews.apache.org/r/44945/diff/
> 
> 
> Testing
> -------
> 
> Make check. Manual verification.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Reply via email to