Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Decouple style change and transitions update #15086

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pozdnyakov
Copy link
Contributor

Update transitions in a separate call chain in order to avoid
infinite loop, in case map is modified from within the
transitions update callback.

@pozdnyakov pozdnyakov added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Jul 9, 2019
@pozdnyakov pozdnyakov requested a review from julianrex July 11, 2019 12:43
@pozdnyakov pozdnyakov self-assigned this Jul 11, 2019
@julianrex
Copy link
Contributor

@pozdnyakov I think we talked about this - but this looks good. Anything else need to happen before merging? Tests?

@pozdnyakov pozdnyakov removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Sep 17, 2019
@pozdnyakov pozdnyakov marked this pull request as ready for review September 17, 2019 12:38
@pozdnyakov
Copy link
Contributor Author

@pozdnyakov I think we talked about this - but this looks good. Anything else need to happen before merging? Tests?

Maybe a test proving that infinite loop problem is solved, otherwise it just should not break the existing behavior 😄

@pozdnyakov pozdnyakov added bug Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS labels Sep 17, 2019
@julianrex
Copy link
Contributor

Turns out we do have a test that fails 😄 Integration Test Harness's testInterruptingAndResetNorthOnlyOnceInIsChanging.

What would be a good thing to specifically test this PR?

@pozdnyakov pozdnyakov force-pushed the mikhail_decouple_map_change_and_transition_updates branch from a53d381 to d2242a5 Compare October 22, 2019 12:08
Update transitions in a separate call chain in order to avoid
infinite loop, in case map is modified from within the
transitions update callback.
@pozdnyakov pozdnyakov force-pushed the mikhail_decouple_map_change_and_transition_updates branch from d2242a5 to 148cfe9 Compare October 22, 2019 12:13
@chloekraw
Copy link
Contributor

@pozdnyakov 👋 hey, what's blocking this / what are next steps? Is it waiting for re-review from the iOS team?

@julianrex
Copy link
Contributor

@pozdnyakov is this PR still in flight? I still think it would be a good addition.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants