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
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
