URL: https://github.com/SSSD/sssd/pull/5693 Title: #5693: Basics of 'subid ranges' support for IPA provider
pbrezina commented: """ > Hi @pbrezina, > > thank you for the review. > > > * Please, provide some description to the first commit message. > > Do you think it makes sense to keep 2 commits? I was going to squash them > after review. It's up to you, just make sure it has sufficient description. > > * Why conditional build? Why is build disabled by default? > > This is MVP of experimental feature. > Corresponding code in shadow-utils is merged upstream, but there is no > upstream release available yet. We brought functionality to Fedora via > patches. But I doubt other distributions will do the same. I.e. out of > Fedora/RHEL, this is at the moment for enthusiasts who are willing to play > with a new feature and who is capable to rebuild shadow-utils from sources. > Moreover, I anticipate significant changes might be required once we have > initial feedback. > > So... at the very least it's conditional to allow distributions to avoid > installing plugin that they can't use yet. Ack. This is something to document in the commit message. > Once shadow-utils upstream release is done and widely used, Podman patches > released > ([containers/storage#882](https://github.com/containers/storage/pull/882)), > etc (so it is more or less easy to use), we can change default and build code > / install plugin in sssd-ipa, IMO. Ok. > > ``` > > * We don't use two blank lines between function definitions. > > ``` > > And this (monolithic text) makes me sad :) I didn't see such restrictions in > https://sssd.io/contrib/coding-style.html :-) Even though something is not explicitly banned, it is not cool to differentiate from the rest of the project. > > ``` > > * Can the range be set also for trusted domain user? If yes, this > > implementation doesn't seem to support it. > > ``` > > No, FreeIPA doesn't support it in MVP. Code looks good to me. I'm going to test it. """ See the full comment at https://github.com/SSSD/sssd/pull/5693#issuecomment-887410280
_______________________________________________ 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 Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
