Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Resolve Internal Inconsistency During Updates When Using Stable Range Controller#1060

Merged
appleguy merged 1 commit intofacebookarchive:masterfrom
Adlai-Holler:FixRangeControllerUpdateIssue
Jan 14, 2016
Merged

Resolve Internal Inconsistency During Updates When Using Stable Range Controller#1060
appleguy merged 1 commit intofacebookarchive:masterfrom
Adlai-Holler:FixRangeControllerUpdateIssue

Conversation

@Adlai-Holler
Copy link
Copy Markdown
Contributor

This addresses #1028 #1055 #1047 .

The issue seems to be caused by one of the changes in ffcddf3 where we stopped checking if the range was valid before removing nodes from the working range. I just reverted that change exactly as-was and the issue is gone for me.

@appleguy I'm not sure the implications of this change. I suspect it means that under certain conditions, nodes won't be removed from the working range. We may now be throwing the baby out with the bathwater.

@Adlai-Holler
Copy link
Copy Markdown
Contributor Author

Ah I just saw #1054 . This may work as a hotfix.

@appleguy
Copy link
Copy Markdown
Contributor

@Adlai-Holler thank you for debugging my regression. I also appreciate you making a minimized change, as this code has actually already been tested (although granted, not with every more recent change). I'll push out a new dot release shortly, probably later tonight.

appleguy added a commit that referenced this pull request Jan 14, 2016
Resolve Internal Inconsistency During Updates When Using Stable Range Controller
@appleguy appleguy merged commit d56f0b1 into facebookarchive:master Jan 14, 2016
@appleguy appleguy added the Bug label Jan 14, 2016
@appleguy appleguy added this to the 1.9.5 milestone Jan 14, 2016
@knopp
Copy link
Copy Markdown
Contributor

knopp commented Jan 14, 2016

@appleguy, with this fix will break #1011 again :-/ Although I guess it is better than crashing if you don't want to merge #1054 for some reason

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants