On (10/06/16 11:01), Martin Kosek wrote: >On 06/10/2016 10:01 AM, Martin Basti wrote: >> >> >> On 09.06.2016 21:45, Alexander Bokovoy wrote: >>> On Thu, 09 Jun 2016, Martin Basti wrote: >>>> >>>> >>>> On 09.06.2016 17:56, Martin Babinsky wrote: >>>>> On 06/06/2016 01:37 PM, Alexander Bokovoy wrote: >>>>>> On Mon, 06 Jun 2016, Jan Cholasta wrote: >>>>>>> On 6.6.2016 13:22, Martin Basti wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 06.06.2016 13:14, Alexander Bokovoy wrote: >>>>>>>>> On Mon, 06 Jun 2016, Martin Basti wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 06.06.2016 12:36, Alexander Bokovoy wrote: >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> MS-ADTS spec requires that TrustPartner field should be equal to the >>>>>>>>>>> commonName (cn) of the trust. We used it a bit wrongly to express >>>>>>>>>>> trust relationship between parent and child domains. In fact, we >>>>>>>>>>> have parent-child relationship recorded in the DN (child domains >>>>>>>>>>> are part of the parent domain's container). >>>>>>>>>>> >>>>>>>>>>> Remove the argument that was never used externally but only >>>>>>>>>>> supplied by >>>>>>>>>>> trust-specific code inside the IPA framework. >>>>>>>>>>> >>>>>>>>>>> Part of https://fedorahosted.org/freeipa/ticket/5354 >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Hello, how is handled backward compatibility here, you just removes >>>>>>>>>> the option from API, without any additional logic for older clients. >>>>>>>>> This is not used by the external clients at all. It is part of >>>>>>>>> internal >>>>>>>>> logic of the code in trust.py+com.redhat.trust.fetch-domains which >>>>>>>>> always talk to the same server they are running on. >>>>>>>>> >>>>>>>>> @register() >>>>>>>>> class trustdomain_add(LDAPCreate): >>>>>>>>> __doc__ = _('Allow access from the trusted domain') >>>>>>>>> NO_CLI = True >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> Yes sorry, not old IPA clients, but it was part of API, shown in API >>>>>>>> browser, and since this was in API, it is set to stone. So If you think >>>>>>>> that it is safe to be removed and nobody can hit this, I'm okay for >>>>>>>> removing that option. Maybe we should at least wrote it to release >>>>>>>> notes >>>>>>>> (I'll let Honza to express his feelings as API versioning/compatibility >>>>>>>> sensei) >>>>>>> >>>>>>> IMHO it is safe to remove. >>>>>>> >>>>>>>> >>>>>>>> And you forgot to increment api version in VERSION file >>>>>> Updated patch attached, with a VERSION change. >>>>>> >>>>>> >>>>>> >>>>> ACK >>>>> >>>> >>>> Is there any ticket for this? >>> As I wrote in the commit message and in the email, >>> it is part of https://fedorahosted.org/freeipa/ticket/5354 >>> >> Sorry I misread that ticket in the commit message, because ipatool was unable >> to parse it from commit message >> >> Pushed to master: 185806432d6dfccc5cdd73815471ce60a575b073 > >I see no link to this ticket in the commit message in >https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=185806432d6dfccc5cdd73815471ce60a575b073 >Did you push old version of this patch? > >In general, I would suggest using the patch format from >http://www.freeipa.org/page/Contribute/Patch_Format >It makes automation easier... > And it would be much easier for author with .git-commit-template @see https://git.fedorahosted.org/cgit/sssd.git/commit/?id=3d9edb4c510028def2df41aa7b0ce705b197e6fc
LS -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
