-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44945/#review125457
-----------------------------------------------------------
configure.ac (line 258)
<https://reviews.apache.org/r/44945/#comment188243>
Two spaces to the right.
Also, append "default: no"
configure.ac (line 259)
<https://reviews.apache.org/r/44945/#comment188248>
We probably shouldn't take option arugments, i.e., `--enable-xfs-disk-isolator` is sufficient.
Otherwise there is confusion with `--enable-xfs-disk-isolator=yes|true|1|absolutely`. :)
s/[enable_xfs_disk_isolator=no]/[] for consistency.
configure.ac (line 930)
<https://reviews.apache.org/r/44945/#comment188258>
Kill this blank line.
Also, I know the headers below imply Linux but I guess it won't hurt to add
```
AS_IF([test "$OS_NAME" = "linux"],
[],
[AC_MSG_ERROR([cannot build xfs disk isolator
-------------------------------------------------------------------
XFS disk isolator is only supported on Linux.
-------------------------------------------------------------------
])])
```
so the error message is clearer.
configure.ac (line 934)
<https://reviews.apache.org/r/44945/#comment188262>
Move one space to the right.
configure.ac (line 954)
<https://reviews.apache.org/r/44945/#comment188263>
This could be the place we insert
```
AC_DEFINE([ENABLE_XFS_DISK_ISOLATOR])
```
configure.ac (lines 957 - 960)
<https://reviews.apache.org/r/44945/#comment188321>
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])`
configure.ac (lines 962 - 963)
<https://reviews.apache.org/r/44945/#comment188322>
If we pull this up into the aforementioned place we don't need to `test "x$enable_xfs_disk_isolator"
= "xyes"` repeatedly and therefore be more concise.
- Jiang Yan Xu
On March 22, 2016, 4:21 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44945/
> -----------------------------------------------------------
>
> (Updated March 22, 2016, 4:21 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
>
>
|