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

Support Multi-Attribute Strings #6

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

gregpardo
Copy link

No description provided.

Greg Pardo and others added 5 commits October 13, 2015 12:43
Add list of methods and explain how to build an AttributedString with various attributes at ranges
Clarify default of clearsAttributesOnNextString
@SRandazzo SRandazzo changed the title Develop Support Multi-Attribute Strings Oct 20, 2015
public func setAttribute(attribute: NSAttributedStringAttribute) {
// Builder functions

public func setAttribute(attribute: NSAttributedStringAttribute) throws {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you explain why this function throws?

@SRandazzo
Copy link
Owner

A++ PR 👍

Unit tests on Jenkins appear to be failing. One failure is related to the addition of throws which I commented on

If you can get those passing, and add additional coverage for the new methods we should be good to go 👍 Great job

For the tests, you'll want to run them on the iPhone 6 iOS 9.0 simulator because of SnapShot test consistency.

You should add a SnapShot test for your new addition, they are really easy and fun to write! Do let me know if you need any help getting started, there are some current tests in VeronaTests.swift

Thanks again!

https://travis-ci.org/SRandazzo/Verona/builds/85341853

@SRandazzo
Copy link
Owner

@gregpardo Just wanted to bump as I got a little side tracked and forgot about this!

Everything looks super great here, if you resolve those couple of CR notes you are good to merge! I'll pull you in as a contributor to the repo :-D Thanks again for the great work

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.

2 participants