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

detach() or detatch() ? #404

Closed
damienflament opened this issue Apr 16, 2017 · 3 comments
Closed

detach() or detatch() ? #404

damienflament opened this issue Apr 16, 2017 · 3 comments

Comments

@damienflament
Copy link

The current documentation about the ComponentUI mention a detach() method. But this method is called detatch in the source code and in the specification also.

@anthonyjb
Copy link
Member

@damienflament should be detach - misspelled in the source - I'll upload a patch early this week -ty 👍

@damienflament
Copy link
Author

damienflament commented Apr 17, 2017

Isn't it a source of serrious bugs ? Even if you're not aware of them (because the detach method is defined in the "subclasses" of your projects) ?

@anthonyjb
Copy link
Member

@damienflament the detatch\detach method isn't called in the ContentTools libraries, though other external libraries may use it.

The patch I implement will retain the existing detatch method with a warning recommendation output in the console to switch to the correct detach method by the next minor update. This will mean the change has no impact on existing projects and the version change will be a patch 1.3.2 -> 1.3.3.

I don't consider this a serious bug as the modification will be backwards compatible and the existing detatch will remain until the next minor update (major.minor.patch).

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

No branches or pull requests

2 participants