-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
replacing blacklist by blocklist and whitelist by allowlist #744
Conversation
Signed-off-by: Andreas Pre <aponb@gmx.at>
✅ Gradle Wrapper Validation success d8f325c |
✅ Gradle Precommit success d8f325c |
✅ DCO Check Passed d8f325c |
This pull request supersedes #729. |
@aponb Thanks very much for doing this! I really appreciate it. As @dblock said, we're trying really hard not to introduce any breaking changes in 1.0 (which is why we're moving changes like #472 to 2.0). But these are absolutely the changes I want to make, because this is the kind of software (inclusive) that we want to build. So thanks again. :) |
@aponb To get this merged you would need to add fallback for the settings to the old names in code. For example I would also suggest maybe splitting the work. We could first merge a change that changes identifiers but not actual things like file names to preserve backwards compatibility. That would significantly reduce the amount of places we see these keywords in the codebase, and the work we'd have to do will be lesser later.
When we have only actual backwards compatibility questions, we can take them 1-by-1. For example, I am not sure Does this help? |
Sorry for the late response. That is definitely a way to go. In a first shot I am going to remove all old names from the code base. Then we can look out for the rest and do some fallback scenarios. |
✅ DCO Check Passed d8f325c |
✅ Gradle Wrapper Validation success d8f325c |
✅ Gradle Precommit success d8f325c |
@aponb I hope you find time to pick this up! |
Yes, I am going to do it in the next days! |
Can one of the admins verify this patch? |
@aponb Hey! Just checking in to see if you've had a chance to take a look at this. Thanks! |
sorry for being so late. Going to pick up that topic in the next weeks! |
Hi @aponb, we have got a plan to replace the non-inclusive term "blacklist / whitelist" (#1483), could you please take a look at the plan and modify your PR?
Look forward to hearing from you soon. Thank you! 👍 |
ok thanks, I will stick to that plan. Probably I will have new Pull Requests, as this one is harder to change. Keep you informed! |
@aponb Thank you for your help and your understanding! 😁 👍 . |
Hi @aponb, could you please share any plan or progress on this issue? Thank you! |
I could make a new pull request with changes of replacing the exclusionary words that won't impact the compatibility. Changes just in code comments, internal variables, methods and class names. In doubt I kept the old wording. But a lot of the words are gone that way. I you want I could post the new pull request. |
Hi @aponb, that sounds good! Look forward to see your new pull request soon. 😄 Let's start from replacing the non-inclusive term "blacklist/whitelist" in code comments or internal variable/method/class names, and then if you have time, you could go on following our plan. |
@tlfeng What should we do with this PR? |
Signed-off-by: Andreas Pre aponb@gmx.at
Description
opensearch should be open enough to avoid the words "blacklist" and "whitelist"
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.
Signed-off-by: Andreas Pre aponb@gmx.at