Skip to content
This repository has been archived by the owner on Dec 11, 2020. It is now read-only.

Added 'red' to $safeColorNames #1701

Merged

Conversation

xfudox
Copy link

@xfudox xfudox commented May 22, 2019

No description provided.

Copy link
Contributor

@localheinz localheinz left a comment

Choose a reason for hiding this comment

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

👍

@pimjansen pimjansen merged commit 3f74f49 into fzaninotto:master Aug 27, 2019
@fzaninotto
Copy link
Owner

@localheinz @pimjansen This is the kind of change that I usually refuse. The reason is that many people use Faker for unit tests (with a fixed seed). Whenever we change a dataset, these unit tests break. We should only do it for valid reasons. Adding one color (or one name, or one city) isn't a good enough reason.

If this dataset is incomplete, then we need a PR with a complete dataset, that doesn't risk being updated in the near future. I summarize it by saying "a change in a dataset should bring a large enough added value".

@xfudox I'm reverting your merge for the reasons details above. Thanks for your contribution anyway.

@pimjansen
Copy link
Contributor

Ok fair enough 👌🏻

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants