-
Notifications
You must be signed in to change notification settings - Fork 490
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
Replace "whitelist/blacklist" with "allowed/blocked" #591
base: master
Are you sure you want to change the base?
Conversation
…or of allowed/blocked
…cts to allowed_metricsRejects
…r to see progress
I'm pushing this back to the next milestone for now. I would like to see this go in but after working on it for a day I feel like there's room for improvement in terms of naming clarity. I've gone through it a few times and caught myself getting confused with some of the functions/settings. |
Maybe a unit test to ensure that backward compatibility works ? I know that if we end up breaking it quite a lot of production systems might just burst into flames. |
This is off topic, but - I'm helping colleagues set up graphite & friends, and came across this PR (and related threads). I wanted to say that the maintainers did a great job managing the (distracting) side conversations. In addition, I've been trying to come up with replacement words for black/whitelist, and your choices are great! Thank you for this wonderful project. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Looks like tests will require some massage |
This PR replaces #568, fixing a merge conflict.
ref #567