[sssd PR#81][opened] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Author: jhrozek
 Title: #81: Please see the commit message, the fix is hopefully simple
Action: opened

PR body:
"""
None
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/81/head:pr81
git checkout pr81

[sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

lslebodn commented:
"""
This is only a problem with strict warning flags.
which is usually a develer build.

There is not a problem in downstream centos/fedora
->see https://kojipkgs.fedoraproject.org//packages/sssd/1.14.2/1.fc24/data/logs/x8...

```
checking sys/inotify.h usability... yes
checking sys/inotify.h presence... yes
checking for sys/inotify.h... yes
checking whether sys/inotify.h actually works... yes
```

And more verbose version:
```
configure:25860: checking sys/inotify.h usability
configure:25860: gcc -c -g -O2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-D_LARGEFILE64_SOURCE  conftest.c >&5
configure:25860: $? = 0
configure:25860: result: yes
configure:25860: checking sys/inotify.h presence
configure:25860: gcc -E  conftest.c
configure:25860: $? = 0
configure:25860: result: yes
configure:25860: checking for sys/inotify.h
configure:25860: result: yes
configure:25871: checking whether sys/inotify.h actually works
configure:25883: gcc -o conftest -g -O2 -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE   conftest.c  >&5
conftest.c: In function 'main':
conftest.c:186:19: warning: implicit declaration of function 'inotify_init'
[-Wimplicit-function-declaration]
     return (-1 == inotify_init());
                        ^~~~~~~~~~~~
configure:25883: $? = 0
configure:25884: result: yes
```

Anyway,
have you tested your patch?
I can still see issue there.
```
configure:25883: gcc -o conftest -g -O2 -ftrack-macro-expansion=0 -Wextra -Wno-unused-parameter -Wno-sign-compare -Werror -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE  -Wl,--as-needed conftest.c  >&5
conftest.c:183:25: error: extra tokens at end of #include directive [-Werror]
 #include <sys/inotify.h>,
                         ^
cc1: all warnings being treated as errors
configure:25883: $? = 1
```

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/81#issuecomment-261513452

[sssd PR#81][+Changes requested] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

Label: +Changes requested

[sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

fidencio commented:
"""
So, AFAIU what's is missing is this small patch:

```
[ffidenci@cat x86_64]$ git diff
diff --git a/src/external/inotify.m4 b/src/external/inotify.m4
index bcf9408..25259a8 100644
--- a/src/external/inotify.m4
+++ b/src/external/inotify.m4
@@ -7,7 +7,7 @@ AC_DEFUN([AM_CHECK_INOTIFY],
     AC_LINK_IFELSE(
         [AC_LANG_SOURCE([
 #ifdef HAVE_SYS_INOTIFY_H
-#include <sys/inotify.h>,
+#include <sys/inotify.h>
 #endif
 int main () {
     return (-1 == inotify_init());
```

Feel free to use the gist above as a different patch (I don't care about the ownership).

For future interactions would be way simpler if the reviewer could just push the patch with the simple fix and point it in the review instead of having it blocked here for a few days.
"""

See the full comment at https://github.com/SSSD/sssd/pull/81#issuecomment-261929087

[sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

lslebodn commented:
"""
On (21/11/16 13:53), fidencio wrote:
> For future interactions would be way simpler if the reviewer could just push the patch with the simple fix and point it in the review instead of having it blocked here for a few days.

A) It is not a hight priority or critical fix; so nothing was blocked.

B) In this case, the fixup change was simple. But there is still question
which changes are approrpiate to be done before push which are not.
The author of the patch need't agree with some changes.

It would be OK if author wrote please change that before pushing the patch.
But it would take the same amount of time as changing code and push it.

LS

"""

See the full comment at https://github.com/SSSD/sssd/pull/81#issuecomment-261933794

[sssd PR#81][synchronized] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Author: jhrozek
 Title: #81: Please see the commit message, the fix is hopefully simple
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/81/head:pr81
git checkout pr81

[sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

jhrozek commented:
"""
Thanks for the review, I pushed a new version. I did test the patch (otherwise the conditional build of the files provider where I added AM_CONDITIONAL didn't work) but I have no idea how it could work :)

Anyway, let's see if this version is OK..
"""

See the full comment at https://github.com/SSSD/sssd/pull/81#issuecomment-262228673

[sssd PR#81][-Changes requested] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

Label: -Changes requested

[sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

fidencio commented:
"""
Acked-by: Fabiano Fidêncio 
"""

See the full comment at https://github.com/SSSD/sssd/pull/81#issuecomment-262235759

[sssd PR#81][+Accepted] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

Label: +Accepted

[sssd PR#81][comment] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

lslebodn commented:
"""
master:
* 2927dc45b9bc810f4f55bce165bb96405129e693

sssd-1-14:
* 495289cfa922b00278aa91d433489403e792304e

sssd-1-13:
* 331a7c3d60a4dadd43565f339095daef5de5e7bf

"""

See the full comment at https://github.com/SSSD/sssd/pull/81#issuecomment-262269392

[sssd PR#81][+Pushed] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Title: #81: Please see the commit message, the fix is hopefully simple

Label: +Pushed

[sssd PR#81][closed] Please see the commit message, the fix is hopefully simple

[fidencio]

URL: https://github.com/SSSD/sssd/pull/81
Author: jhrozek
 Title: #81: Please see the commit message, the fix is hopefully simple
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/81/head:pr81
git checkout pr81