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

Add ellipse mask type #481

Merged
merged 3 commits into from May 17, 2017

Conversation

Projects
None yet
4 participants
@phimage
Member

phimage commented May 14, 2017

Add missing triangle in demo

First attempt on MaskType
I have others mask to propose, I will make an issue
#482

@phimage phimage requested a review from JakeLin May 14, 2017

@IBAnimatableBot

This comment has been minimized.

Show comment
Hide comment
@IBAnimatableBot

IBAnimatableBot May 14, 2017

1 Warning
⚠️ Consider adding supporting documentation to this change. Documentation can be found in the docs directory.

Generated by 🚫 Danger

IBAnimatableBot commented May 14, 2017

1 Warning
⚠️ Consider adding supporting documentation to this change. Documentation can be found in the docs directory.

Generated by 🚫 Danger

@JakeLin

Nice one, and with UI tests in the demo app 👍

@JakeLin

This comment has been minimized.

Show comment
Hide comment
@JakeLin

JakeLin May 15, 2017

Member

@phimage nice one, looking forward to seeing the sharps in #482 too. Can you please update the CHANGLOG.md or you want to treat it as one item for all masks in #482? Please merge it when you free ready, thanks 👍

Member

JakeLin commented May 15, 2017

@phimage nice one, looking forward to seeing the sharps in #482 too. Can you please update the CHANGLOG.md or you want to treat it as one item for all masks in #482? Please merge it when you free ready, thanks 👍

@tbaranes

This comment has been minimized.

Show comment
Hide comment
@tbaranes

tbaranes May 15, 2017

Member

Travis is failing because Danger has reported "CHANGELOG missing", once the entry added, travis should be green!

Member

tbaranes commented May 15, 2017

Travis is failing because Danger has reported "CHANGELOG missing", once the entry added, travis should be green!

@phimage

This comment has been minimized.

Show comment
Hide comment
@phimage

phimage May 15, 2017

Member

I know, no PR could be done in one commit, even the little one 😭

And when I will merge this one with CHANGELOG update, I have some probability of having conflict with #478 💀
So I will wait for 478 merge

Member

phimage commented May 15, 2017

I know, no PR could be done in one commit, even the little one 😭

And when I will merge this one with CHANGELOG update, I have some probability of having conflict with #478 💀
So I will wait for 478 merge

@tbaranes

This comment has been minimized.

Show comment
Hide comment
@tbaranes

tbaranes May 15, 2017

Member

If you really want only one commit, you can amend the changelog update in your branch, then force push. It will make this PR ready to go with one commit only!

Member

tbaranes commented May 15, 2017

If you really want only one commit, you can amend the changelog update in your branch, then force push. It will make this PR ready to go with one commit only!

@phimage phimage added this to the 5.0 milestone May 17, 2017

@phimage phimage merged commit 0fcccaa into master May 17, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@phimage phimage deleted the feature/ellipse-mask branch May 17, 2017

@JakeLin

This comment has been minimized.

Show comment
Hide comment
@JakeLin

JakeLin May 20, 2017

Member

@phimage well done 🎉🎉🎉

Member

JakeLin commented May 20, 2017

@phimage well done 🎉🎉🎉

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