Skip to content

Add RGB, ARGB and RGBA methods for the category of UIColor. - #65

Merged
qcl merged 6 commits into
yahoo:masterfrom
joehsieh:AddExtendingColorMethods
Oct 25, 2017
Merged

Add RGB, ARGB and RGBA methods for the category of UIColor.#65
qcl merged 6 commits into
yahoo:masterfrom
joehsieh:AddExtendingColorMethods

Conversation

@joehsieh

Copy link
Copy Markdown
Contributor

No description provided.

@yahoocla

Copy link
Copy Markdown

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! 😄

@codecov-io

codecov-io commented Oct 23, 2017

Copy link
Copy Markdown

Codecov Report

Merging #65 into master will increase coverage by 0.78%.
The diff coverage is 50.7%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #65      +/-   ##
==========================================
+ Coverage   24.94%   25.72%   +0.78%     
==========================================
  Files          29       29              
  Lines        2169     2239      +70     
  Branches      320      320              
==========================================
+ Hits          541      576      +35     
- Misses       1589     1624      +35     
  Partials       39       39
Impacted Files Coverage Δ
...vPods/AppDevCommonKit/UIColor+ADKHexPresentation.m 52.34% <50.7%> (-2.83%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51b011d...509892c. Read the comment docs.

@qcl qcl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd done my review. And I hope you can also complete the test for those new methods.

/**
* @brief Create a color by hex string.
* @brief Create a color by hex string. The returned color is undefined if hexString is invalid.
* The behaviour is the same as ADKColorWithRGBHexString:, but ADKColorWithRGBHexString is preferred.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Second ADKColorWithRGBHexString missed its :.

@joehsieh

Copy link
Copy Markdown
Contributor Author

I've fixed the typo and applied related tests.

@joehsieh

Copy link
Copy Markdown
Contributor Author

BTW, I also upgrade IPHONEOS_DEPLOYMENT_TARGET of test target from 7.0 to 8.0 to build the tests successfully.

@qcl qcl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd done my review.


SPEC_END No newline at end of file
describe(@"Test ADKColorWithRGBHexString", ^{
it(@"given color with 7b0099, should get string 7B0099", ^{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It should be given string 7B0099, should get color with 7b0099.

});

describe(@"Test ADKColorWithARGBHexString", ^{
it(@"given color with EE7b0099, should get string EE7B0099", ^{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ditto.

});

describe(@"Test ADKColorWithRGBAHexString", ^{
it(@"given color with 7b0099EE, should get string 7B0099EE", ^{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ditto.

@joehsieh

Copy link
Copy Markdown
Contributor Author

I've updated the test description. Thanks.

@qcl qcl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@qcl
qcl merged commit fc0c48c into yahoo:master Oct 25, 2017
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