Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX release] fix issue with unchaining ChainNodes #15849

Merged
merged 2 commits into from
Nov 12, 2017

Conversation

bekzod
Copy link
Contributor

@bekzod bekzod commented Nov 11, 2017

fix for #15845 #15836

@rwjblue
Copy link
Member

rwjblue commented Nov 11, 2017

Can you cherry pick the failing test commits here so that we can confirm they are passing?

@rwjblue
Copy link
Member

rwjblue commented Nov 12, 2017

Can you confirm that the regression this fixes is in 2.16? If so, can you prefix the fixing commit with [BUGFIX release] so we know to include it in the next patch release?

@bekzod bekzod changed the title fix issue with unchaining ChainNodes [BUGFIX release] fix issue with unchaining ChainNodes Nov 12, 2017
@bekzod
Copy link
Contributor Author

bekzod commented Nov 12, 2017

done

@bekzod bekzod closed this Nov 12, 2017
@bekzod bekzod reopened this Nov 12, 2017
@bekzod
Copy link
Contributor Author

bekzod commented Nov 12, 2017

accidentally closed it :P

@rwjblue rwjblue merged commit 5b3a22c into emberjs:master Nov 12, 2017
@rwjblue
Copy link
Member

rwjblue commented Nov 12, 2017

Thank you!

@krisselden
Copy link
Contributor

This is poor root cause analysis, remove should never be called unbalanced, we still don't know the cause of the original regression this is just masking the symptom.

@krisselden
Copy link
Contributor

we need to find the commit that caused the regression, this fix is just hiding the issue, we must have changed something around chains being overriden during extend or finish chains

@krisselden
Copy link
Contributor

@bekzod can you git bisect to find the regressing commit? this fix is no good, it is just hiding the unbalanced watch/unwatch. This is going to cause other more subtle bugs.

@bekzod
Copy link
Contributor Author

bekzod commented Dec 13, 2017

@krisselden roger that

@bekzod
Copy link
Contributor Author

bekzod commented Dec 13, 2017

found offending commit 0c72faa

@bekzod
Copy link
Contributor Author

bekzod commented Dec 13, 2017

will checkout why it is happening later today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants