Skip to content

Comments

Fix: Allow required initializator methods @ Views.#905

Merged
zhongwuzw merged 1 commit intoMessageKit:developmentfrom
talanov:development
Oct 14, 2018
Merged

Fix: Allow required initializator methods @ Views.#905
zhongwuzw merged 1 commit intoMessageKit:developmentfrom
talanov:development

Conversation

@talanov
Copy link

@talanov talanov commented Oct 11, 2018

What does this implement/fix? Explain your changes.

Removes crash, if calling AvatarView from another XIB.

Does this close any currently open issues?

No.

Any relevant logs, error output, etc?

Crash: Line 156 @ AvatarView.swift

Any other comments?

Nope.

Where has this been tested?

Devices/Simulators: iPx, iPxr, iPxs, iP6, iP6+

iOS Version: 11-12

Swift Version: 4.2

MessageKit Version: Last

Copy link
Member

@zhongwuzw zhongwuzw left a comment

Choose a reason for hiding this comment

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

💯

@zhongwuzw
Copy link
Member

@talanov Please add a CHANGELOG entry in CHANGELOG.md

@talanov
Copy link
Author

talanov commented Oct 11, 2018

@zhongwuzw

Thanks for accepting pull request!

I don't think it's really worth mentioning, since it's 1 line of code. :)

talanov pushed a commit to talanov/MessageKit that referenced this pull request Oct 11, 2018
@talanov
Copy link
Author

talanov commented Oct 11, 2018

OK, now it's 2 lines of code.

@nathantannar4
Copy link
Member

Are there any other init(aDecoder: Decoder) methods that should be added? I thought we already fixed this

Copy link
Member

@zhongwuzw zhongwuzw left a comment

Choose a reason for hiding this comment

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

Please see my comments.

@talanov
Copy link
Author

talanov commented Oct 12, 2018

Fixed other similar issues.

@nathantannar4
Copy link
Member

@talanov Thanks for finding those, however a method should be extracted for each unit to set up the view rather than copy/paste

Sent with GitHawk

@nathantannar4
Copy link
Member

@talanov If you could make those changes today that would be awesome so we could include it in the next release tomorrow!

@talanov
Copy link
Author

talanov commented Oct 13, 2018

@nathantannar4 Done.

@talanov talanov changed the title HotFix: Allow required init coder @ Avatar View. Fix: Allow required initializator methods @ Views. Oct 13, 2018
CHANGELOG.md Outdated

- Fixed `MessagesCollectionView` to allow to use nibs with `MessageReusableView`.
[#832](https://github.com/MessageKit/MessageKit/pull/832) by [@maxxx777](https://github.com/maxxx777).
- Fixed crash at `AvatarView`, when `AvatarView` is being called from another XIB. [#905](https://github.com/MessageKit/MessageKit/pull/905) by [@talanov](https://github.com/talanov).
Copy link
Member

Choose a reason for hiding this comment

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

Please update the CHANGELOG.

Copy link
Member

@nathantannar4 nathantannar4 left a comment

Choose a reason for hiding this comment

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

Just amend the CHANGELOG to reflect that multiple crash fixes were made

@talanov
Copy link
Author

talanov commented Oct 14, 2018

Done.

Copy link
Member

@zhongwuzw zhongwuzw left a comment

Choose a reason for hiding this comment

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

@talanov Please see my comment, and could you squash all these commits to one?

@talanov talanov force-pushed the development branch 2 times, most recently from 67a5e2c to 3040d15 Compare October 14, 2018 14:41
…ayButton Views, MessageContent, MessageCollectionView Cells, MessagesCollectionViewFlow Layout.
@talanov
Copy link
Author

talanov commented Oct 14, 2018

@nathantannar4 @zhongwuzw

Rebased. Done.

@SD10
Copy link
Member

SD10 commented Oct 14, 2018

Thank you for contributing to MessageKit! I've invited you to join the MessageKit GitHub organization - no pressure to accept! If you'd like more information on what that means, check out our contributing guidelines and join the MessageKit Slack channel. Feel free to reach out if you have any questions! 😃

@zhongwuzw zhongwuzw merged commit aa734da into MessageKit:development Oct 14, 2018
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