URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
Pushed in ef39016..e083a6b
"""
See the full comment at
https://github.com/SSSD/sssd/pull/34#issuecomment-255036044
___
sssd-devel mail
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
On Wed, Oct 19, 2016 at 02:48:57AM -0700, Jakub Hrozek wrote:
> Thank you, ACK. I sent the patch to CI and I will push the patch when CI
> finishes.
passed: http://sssd-ci.duckdn
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
Thank you, ACK. I sent the patch to CI and I will push the patch when CI
finishes.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/34#issuecomment-254766112
__
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
pbrezina commented:
"""
Updated patch pushed. Diff:
```c
diff --git a/src/responder/common/cache_req/cache_req_search.c
b/src/responder/common/cache_req/cache_req_search.c
index 3c11efd..a36a900 100644
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
Actually, sorry, one more Coverity warning:
```
Error: FORWARD_NULL (CWE-476):
sssd-1.14.2/src/responder/common/cache_req/cache_req_search.c:105:
var_compare_op: Comparing "result
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
Apart from the nitpicks, the code looks better to me (I like that the lookup
functionality is abstracted, so adding new lookups will be easy).
I tested several kinds of lookups,
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
pbrezina commented:
"""
Updated patches pushed.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/34#issuecomment-253188271
___
sssd-devel mailin
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
Well, it's still a lot of code :) but at least I started the review now. For
starters, Coverity found some warnings:
```
Error: FORWARD_NULL (CWE-476):
sssd-1.14.2/src/responder/c
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
pbrezina commented:
"""
I finished the code and split it into four commits:
1. Implements logic ... it is the same as in responder_cache_req.c but with
added support for plugins.
2. Implements plugins .
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
on another note, this commit might be a reason to split 1.14 and master..
"""
See the full comment at
https://github.com/SSSD/sssd/pull/34#issuecomment-250700349
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
Great. The next thing is...is there a way to split the commit a bit more? I
have no idea how to review a 1800+ lines commit..
"""
See the full comment at
https://github.com/SSSD
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
pbrezina commented:
"""
On 09/30/2016 09:21 AM, Jakub Hrozek wrote:
> I'm fine with the approach but I didn't read the code line-by-line.
>
> We should make sure the code coverage remains high.
Good thi
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
I'm fine with the approach but I didn't read the code line-by-line.
We should make sure the code coverage remains high.
"""
See the full comment at
https://github.com/SSSD/sssd/
13 matches
Mail list logo
