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

Random color with Placeholdit #1179

Merged

Conversation

nicolas-brousse
Copy link
Contributor

Add option on Placeholdit module to generate random color for text_color and background_color.

@nicolas-brousse
Copy link
Contributor Author

@vbrazo Hey, do you expect something from me? I notice you assign the merge to me.
If yes, let me know how I could help.

@nicolas-brousse
Copy link
Contributor Author

nicolas-brousse commented May 17, 2018

@vbrazo I didn't notice the conflicting files. I guess it was the reason of the assignment. I resolved it.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 2143

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • 7 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.05%) to 99.642%

Files with Coverage Reduction New Missed Lines %
lib/faker/time.rb 7 100.0%
Totals Coverage Status
Change from base Build 2142: 0.05%
Covered Lines: 2227
Relevant Lines: 2235

💛 - Coveralls

@faker-ruby faker-ruby deleted a comment from coveralls May 18, 2018
@vbrazo
Copy link
Member

vbrazo commented May 18, 2018

Could you also update the CHANGELOG.md and add the title of this PR + your GitHub ID?


def generate_color
format('%06x', (rand * 0xffffff))
end
Copy link
Member

@vbrazo vbrazo May 18, 2018

Choose a reason for hiding this comment

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

I'm not sure why we need this private method since only one part of the system is using it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it because I need this peace of code twice in image method. But I could call it directly in the main method.
It's also to have a lighter method.

Let me know if you want I move it inside image method directly.

Copy link
Member

@vbrazo vbrazo May 18, 2018

Choose a reason for hiding this comment

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

Let's leave it how it looks and we can refactor in the future if needed.

If :random given as value for background_color or text_color in
Placeholdit module, it will generate a random hex color.
@vbrazo vbrazo merged commit 42428f4 into faker-ruby:master May 18, 2018
@vbrazo
Copy link
Member

vbrazo commented May 18, 2018

Looks good. Thanks for contributing 🥇

@vbrazo vbrazo self-requested a review July 19, 2018 01:28
davidmorton0 pushed a commit to davidmorton0/faker that referenced this pull request Jul 12, 2021
* Add option to use random color for Placeholdit

If :random given as value for background_color or text_color in
Placeholdit module, it will generate a random hex color.

* Update Placeholdit documentation

* Update 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.

3 participants