-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Provide public interface alternatives to whitelist/blacklist terminology #2442
Merged
mshibuya
merged 2 commits into
carrierwaveuploader:master
from
grantbdev:gb/list-terminology-option
Feb 14, 2021
Merged
Provide public interface alternatives to whitelist/blacklist terminology #2442
mshibuya
merged 2 commits into
carrierwaveuploader:master
from
grantbdev:gb/list-terminology-option
Feb 14, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add public interface alternative to whitelist so users who prefer not to use that terminology can use allowlist in their own uploader code. To make this change minimally intrusive as a start, the default whitelist methods now call the allowlist methods so either method can be overridden and it will work as expected while maintaining backwards compatibility. https://developers.google.com/style/word-list#blacklist rubocop/rubocop#6464
Add public interface alternative to blacklist so users who prefer not to use that terminology can use blocklist in their own uploader code. To make this change minimally intrusive as a start, the default blacklist methods now call the blocklist methods so either method can be overridden and it will work as expected while maintaining backwards compatibility. https://developers.google.com/style/word-list#blacklist rubocop/rubocop#6464
Thank you for the proposal! I totally agree to merge this change.
|
mshibuya
added a commit
that referenced
this pull request
Feb 20, 2021
mshibuya
added a commit
that referenced
this pull request
Feb 21, 2021
This was referenced Mar 11, 2021
joemsak
pushed a commit
to Kadenze/carrierwave
that referenced
this pull request
Mar 27, 2021
joemsak
pushed a commit
to Kadenze/carrierwave
that referenced
this pull request
Mar 27, 2021
joemsak
pushed a commit
to Kadenze/carrierwave
that referenced
this pull request
Mar 27, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal of this pull request is to give developers the option to use something like allowlist/blocklist terminology with this gem instead of whitelist/blacklist. rubocop/rubocop#6464 has links to the original motivation and many changes to other gems that have been replacing those words.
Since managing allowed content types and extensions is a significant part of the gem's purpose and impacts the code users write, as a starting point the changes in this PR are intended to be minimally intrusive. Overriding the existing methods still works so it is backwards compatible. If there is buy-in from the maintainers, I will gladly contribute deeper changes to the internals and deprecation of the old methods could be considered as well.
Allowlist and blocklist are only starting points here and I'm open to different names that could be more descriptive. The wording in some places could probably be improved if there is consensus on the alternative. For example, I used "allowlisted" for consistency but you may feel as I do that it sounds awkward and it could be clearer as "allowed" or "permitted" in those instances.
In this case, I would consider naming the methods like
allowed_extensions
andprohibited_content_types
if you want to match the wording in the translation file and use language closer to what application users might see.