1:38 p.m.
I've incorporated the code with the changes we discussed and it can be
reviewed. See: https://github.com/keycloak/keycloak/pull/6318. Still need
to update the documentation I'll get to that hopefully tomorrow, if not the
day after.
On Wed, Oct 2, 2019 at 1:57 PM Jon Koops <jonkoops(a)gmail.com> wrote:
Thanks for the quick response. I'll see if I can get the changes
in today.
On Wed, Oct 2, 2019 at 1:50 PM Stian Thorgersen <sthorger(a)redhat.com>
wrote:
>
>
> On Wed, 2 Oct 2019 at 13:37, Jon Koops <jonkoops(a)gmail.com> wrote:
>
>> As far as I am concerned the pull requests can be merged as-is. However
>> we need to make sure to add a 'legacy' promiseType to cover the
transition
>> period where promiseType will default to 'native' to give users the
ability
>> to opt-out of the native promises whilst migrating their code.
>>
>> Depending on what you prefer we can do two things. We either merge the
>> pull requests and make a new task to introduce a 'legacy' promiseType. Or
I
>> can update the pull requests that are open now. Let me know what you think.
>>
>
> Added https://issues.jboss.org/browse/KEYCLOAK-11616 - if you can
> incorporate that in current PRs that would be great.
>
>
>>
>> Related to this discussion there are some files that seem duplicated as
>> I can find the same type definitions for the Keycloak adapter in the new
>> theme as almost an exact copy. What should be done with with these files if
>> we make changes? (the specific file in question is
>>
themes/src/main/resources/theme/keycloak-preview/account/resources/app/keycloak-service/keycloak.d.ts)
>>
>> It also seems that in the respective classes that use said definition
>> that the legacy promises are also still used. We should look into finding a
>> way to migrate this code as well. Perhaps including Keycloak as an NPM
>> dependency would make some sense here as well.
>>
>
> This is there temporarily for the new account console and the team
> working on the new account console is already aware that they need to
> remove this duplicate.
>
>
>>
>> I don't consider the above observation as part of the migration in
>> question, however it should probably be discussed by the team as well.
>>
>>
>> On Wed, Oct 2, 2019 at 10:54 AM Stian Thorgersen <sthorger(a)redhat.com>
>> wrote:
>>
>>> Updated https://issues.jboss.org/browse/KEYCLOAK-9346 with sub-tasks
>>> to cover what we agreed and moved your two tasks to sub-tasks of this issue.
>>>
>>> I think both your PRs can be merged now, with
>>> https://issues.jboss.org/browse/KEYCLOAK-11608 covering a clearer
>>> message around the deprecation. I can handle that, but need to discuss with
>>> the team around how we go about this first.
>>>
>>> Not sure when we can make this switch, will discuss it with the team on
>>> Friday and let you know.
>>>
>>> On Fri, 27 Sep 2019 at 13:58, Jon Koops <jonkoops(a)gmail.com> wrote:
>>>
>>>> Great! I completely agree that the points outlined are the way
>>>> forward. Let's focus on getting the deprecation message into the
next
>>>> release together with the updated documentation. There are pull requests
>>>> for these changes and I would greatly appreciate a review of them.
>>>>
>>>> I'll also modify the existing pull requests to incorporate the idea
of
>>>> a 'legacy' promise type and refer to it the documentation with
some
>>>> elaboration that in a future version of Keycloak the default will change
to
>>>> 'native' over 'legacy'.
>>>>
>>>> As for the timeline of the changing of the default to 'native'
and
>>>> eventual removal of 'legacy' what would be the preferred moment
to make
>>>> these changes?
>>>>
>>>