-
Notifications
You must be signed in to change notification settings - Fork 705
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
Remove use of blacklist/whitelist terminology in code #2622
Conversation
+1 |
May I suggest instead "GreenList" and "RedList" |
I've never seen these terms used anywhere else. Is there a precedent for their use elsewhere? Using colors like this also relies on some assumptions about what the colors mean. You're relying on someone knowing that (presumably) green means good, allowed, or ok, and red means bad or blocked. Other cultural associations can risk color based terms being confusing for some. |
Red in Asian cultures is more positive and means good luck. |
I think an issue with "GreenList" and "RedList" is that those are not common terms and could create ambiguity, e.g. those could also be used to refer a list of shades of green/red. |
+1 to "block" and "allow". |
Looking at https://english.stackexchange.com/questions/51088/alternative-term-to-blacklist-and-whitelist it seems "Safelist" is a good suggestion to go along with "Blocklist". |
I think in this particular case it might be worth considering a more explicit name for those properties like "invalidPropertyNames" or "forbittenPropertyNames" or something along these lines instead of "PropertyNamesBlockList", especially since they are not even an actual list. |
"Safelist" could be a bit of a loaded term, depending on the context. Block and Allow seems more generic, and applicable in multiple situations and contexts. |
Yes i think that's a good idea. Having those lists be named "AllowedPropertyNamesList" and "ForbiddenPropertyNamesList" or "BlockedPropertyNamesList" are better than plainly replacing it with a different name for those two words. |
Out of interest, please elaborate (perhaps native (UK) English speakers have a specific view on this term compared to non-native speakers.) |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Well if we have an allowed list of APIs, it implies that if it is not on the "Safelist" then it is unsafe to use, rather than just not allowed. |
@mrlacey for consistency, shouldn't it be BlockedList? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or AllowList + BlockList |
Naming really is inconsistent, as already pointed out, and now violates API design principles . The terminology itself is wonky and should be something like AllowedPropertyNames/ BlockedPropertyNames if we keep standard rules. Note that 'List' shouldn't be included on these types of properties. The term ordering also should be such that it sounds correct in English (Naming for sort ordering isn't the convention) Best: In any case at least align the tenses: Alternate 1: Alternate 2: I wouldn't have merged this without some additional fixes. |
Apologize if I merged a bit early. The names should not affect any public APIs. Please feel free to create a new PR with updates. |
🎉 Handy links: |
Description
Motivation and Context
Removes the use of terms with negative racial connotations and replaces them with neutral terms.
How Has This Been Tested?
Changes are limited to comments and property names.
No impact was verified by rebuilding the code and ensuring no unit tests have been broken.
Screenshots (if appropriate):
n/a