URL:
https://github.com/SSSD/sssd/pull/946
Title: #946: INI: sssctl config-check giving the wrong message
mzidek-rh commented:
"""
This refactoring looks overall good to me. Few comments and nitpicks:
1. The last parameter in sss_ini_read_sssd_conf is always CONFDB_FALLBACK_CONFIG. I do not
think we need a parameter for something that will always have the same value.
2. You have additional space between ! and sss_ini_exists
```
if (! sss_ini_exists(init_data)) {
```
3. You use macro PRINT for strings that cannot be translated. For example, nobody can
translate "%s\n":
```
- while (ref_array_get(ra, i, &msg) != NULL) {
- printf("%s\n", msg);
+ while (ref_array_get(ra_success, i, &msg) != NULL) {
+ PRINT("%s\n", msg);
i++;
}
```
This is not really a big issue, but it will result in unnecessary changes in the pot
files, so I think it is better to avoid using PRINT macro for untranslatable strings.
There are more PRINTs like this in this patch.
4. I think it would be good to add sssctl config-check integration test for the case when
no sssd.conf is present and there are some snippets into the src/tests/intg/test_sssctl.py
, but I will leave this to you.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/946#issuecomment-559352983