URL: https://github.com/SSSD/sssd/pull/136 Title: #136: Tlog integration
pbrezina commented: """ Hi, here are some comments. Mostly nitpicks. Can you also share some link how to enable tlog and test this code? * **CACHE_REQ: Rename search_domains_done to search_domains_next_done```** As far as tevent coding style goes `cache_req_search_domains_done` is correct since the request name is `cache_req_search_domains`. Name `cache_req_search_domains_next_done` is incorrect in this context where it serves as the last callback. The tevent coding style should keep the following form: ``` struct tevent_req *request_name_send(...) errno_t request_name_$step(...) void request_name_$stepdone(...) void request_name_done(...) errno_t request_name_recv(..) ``` Where `$step` and `$stepdone` is something that describes this intermediate step. `$stepdone` may be simply `$step_done` where it works or any other string if it works better. But the last callback name must be `request_name_done`. Please, discard this patch. * **CACHE_REQ: Rename done to search_domains_done** Here is the same problem now. But in addtion, you are messing up with the order of the functions where the last callback called is place before another callback. So I propose the following order and name changes: ``` !. cache_req_send 2. cache_req_process_input 3. cache_req_input_parsed 4. cache_req_select_domains 5. cache_req_search_domains 6. cache_req_search_domains_done => cache_req_process_result 7. cache_req_sr_overlay_done => cache_req_done ``` So just change those names and switch order of these two functions. Please, discard this patch and do this change in commit where you create `cache_req_sr_overlay_done`. * **DP: Overlay sessionRecording attribute on initgr** Please remove unneeded parameters instead of silencing the compilator with `(void)req_name; (void)reply;`. Also remove unused `reply` from `dp_req_initgr_pp_nss_notify`. * **CACHE_REQ: Pull sessionRecording attrs from initgr** See previous tevent comment and also please read [this comment](https://github.com/SSSD/sssd/pull/154#issuecomment-284717277) where I explained tevent coding style to Fabiano and I would like to obey it in `cache_req_sr_overlay.c`. The style is mostly about keeping order and proper names of function. This code should read as: ``` cache_req_sr_overlay_send cache_req_sr_overlay_match_users cache_req_sr_overlay_match_all cache_req_sr_overlay_done cache_req_sr_overlay_recv ``` """ See the full comment at https://github.com/SSSD/sssd/pull/136#issuecomment-297006120
_______________________________________________ sssd-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
