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

Rename -[ASCellNode viewModel] to -[ASCellNode nodeViewModel] to avoid collisions #499

Merged
merged 3 commits into from
Aug 7, 2017

Conversation

Adlai-Holler
Copy link
Member

Thanks to @Kaspik and slack users mlilek, jdeguzman, and emre for reporting the issues this collision has caused in their apps.

This is an implementation of a proposal from @Kaspik to rename the field to nodeViewModel. This will be a breaking change for users who have already modified their product code to account for the collision.

As the implementor of the viewModel field, I take responsibility for all the effort y'all went through to adapt to it. Sorry. Hopefully this patch isn't too late to save some of you a lot of effort.

Everyone let me know what you think about this.

@Kaspik
Copy link
Contributor

Kaspik commented Aug 7, 2017

Thank you, that was really fast! 🎉

Copy link
Contributor

@maicki maicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense. Thanks for the change!

Copy link
Member

@nguyenhuy nguyenhuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Adlai-Holler Adlai-Holler merged commit d2adb8f into master Aug 7, 2017
@Adlai-Holler Adlai-Holler deleted the AHRenameViewModelField branch August 7, 2017 18:04
Adlai-Holler added a commit that referenced this pull request Aug 15, 2017
bernieperez pushed a commit to AtomTickets/Texture that referenced this pull request Apr 25, 2018
…d collisions (TextureGroup#499)

* Rename -[ASCellNode viewModel] -> -[ASCellNode nodeViewModel] to avoid breaking existing code

* Update test

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

Successfully merging this pull request may close these issues.

4 participants