Re: [keycloak-dev] Moving to standardized Promises

Great, thanks. Will get some folks that know TypeScript better than me to take a look at it.

On Wed, 2 Oct 2019 at 20:38, Jon Koops <jonkoops(a)gmail.com&gt; wrote: > 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&gt; 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&gt; >> wrote: >> >>> >>> On Wed, 2 Oct 2019 at 13:37, Jon Koops <jonkoops(a)gmail.com&gt; 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&gt; >>>> 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&gt; 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? >>>>>> _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev