Skip to content
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

Jump to bottom

Change terminology to Allowlist and Denylist #7469

Merged
merged 1 commit into from Nov 5, 2019
Merged

Change terminology to Allowlist and Denylist #7469

merged 1 commit into from Nov 5, 2019

Conversation

koic
Copy link
Member

@koic koic commented Oct 29, 2019

Summary

Follow up #6464, #6466, and #6467.

This PR changes a terminology from Whitelist and Blacklist to Allowlist and Denylist.

This change is an obvious breaking change to some cop options. So I'd like to introduce it before RuboCop 1.0 if this change is acceptable.

Other Information

This change has also been made in Rails repository and other repos.
rails/rails#33681

Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@koic koic force-pushed the change_terminology_to_allowlist_and_denylist branch from 4774503 to adfeabe Compare October 29, 2019 17:41
@koic koic force-pushed the change_terminology_to_allowlist_and_denylist branch 3 times, most recently from 43e554c to c55761f Compare October 30, 2019 01:19
@bbatsov
Copy link
Collaborator

bbatsov commented Oct 30, 2019

I agree with the overall premise and the timing of the change, but I think we can do better with the naming - e.g. instead of naming something Allowlist it can be AllowedPatterns or something like this. I'll leave a couple of inline remarks.

bbatsov
bbatsov reviewed Oct 30, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Oct 30, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Oct 30, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Oct 30, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Oct 30, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Oct 30, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Oct 30, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Oct 30, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Oct 30, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
@bbatsov
Copy link
Collaborator

bbatsov commented Oct 30, 2019

@rubocop-hq/rubocop-core Please add your feedback here as well, if you have some. My general point is that we should try to do something more than just auto-replacing the existing problematic names. From my perspective the use of generic names like whitelist/allowlist also has an usability impact, as it's never clear a list of what we're referring to.

koic
koic commented Nov 1, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
@Drenmi
Copy link
Collaborator

Drenmi commented Nov 1, 2019

Please add your feedback here as well, if you have some. My general point is that we should try to do something more than just auto-replacing the existing problematic names. From my perspective the use of generic names like whitelist/allowlist also has an usability impact, as it's never clear a list of what we're referring to.

I have nothing against replacing whitelist and blacklist, but I agree that there are better contextual options in most cases. 👍

@koic koic force-pushed the change_terminology_to_allowlist_and_denylist branch from c55761f to b1c7a41 Compare November 1, 2019 11:33
@koic
Copy link
Member Author

koic commented Nov 1, 2019

I brushed up this PR. I would be happy to get suggestions if you have other good names :-)

@koic koic force-pushed the change_terminology_to_allowlist_and_denylist branch from b1c7a41 to 5b60700 Compare November 1, 2019 11:39
bbatsov
bbatsov reviewed Nov 2, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Nov 2, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
bbatsov
bbatsov reviewed Nov 2, 2019
View reviewed changes
config/default.yml Outdated Show resolved Hide resolved
@koic koic force-pushed the change_terminology_to_allowlist_and_denylist branch from 5b60700 to 3c3833b Compare November 3, 2019 02:41
@koic
Change terminology to Allowlist and Denylist
4008aa7 
### Summary

Follow up rubocop#6464, rubocop#6466, and rubocop#6467.

This PR changes a terminology from `Whitelist` and `Blacklist` to
`Allowlist` and `Denylist`.

This change is an obvious breaking change to some cop options.
So I'd like to introduce it before RuboCop 1.0 if this change is acceptable.

### Other Information

This change has also been made in Rails repository and other repos.
rails/rails#33681
@koic koic force-pushed the change_terminology_to_allowlist_and_denylist branch from 3c3833b to 4008aa7 Compare November 3, 2019 02:44
@bbatsov bbatsov merged commit 8323550 into rubocop:master Nov 5, 2019
@bbatsov
Copy link
Collaborator

bbatsov commented Nov 5, 2019

Thanks!

bquorning added a commit to rubocop/rubocop-rspec that referenced this pull request Nov 5, 2019
@bquorning
Change potentially offensive terminology
9578d90 
I think the few places we use the term "whitelist", we really don't need
to. Changing to use "allowed" etc. instead.

RuboCop did similar changes in
rubocop/rubocop#7469.
@bquorning bquorning mentioned this pull request Nov 5, 2019
Merged
5 tasks
@koic koic deleted the change_terminology_to_allowlist_and_denylist branch November 5, 2019 20:32
movermeyer
movermeyer reviewed Nov 27, 2019
View reviewed changes
config/default.yml
@@ -3840,7 +3842,7 @@ Style/TrivialAccessors:
# Commonly used in DSLs
AllowDSLWriters: false
IgnoreClassMethods: false
Whitelist:
AllowedMethod:
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems that this is missing the trailing s.
Should be AllowedMethods.

PR incoming.

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue: #7530
PR: #7529

sue445 added a commit to sue445/onkcop that referenced this pull request Nov 27, 2019
@sue445
NamePrefixBlacklist has been renamed to ForbiddenPrefixes since r…
2ad80ca 
…ubocop 0.77.0

c.f. rubocop/rubocop#7469
sue445 added a commit to sue445/onkcop that referenced this pull request Nov 27, 2019
@sue445
NamePrefixBlacklist has been renamed to ForbiddenPrefixes since r…
3e013de 
…ubocop 0.77.0

c.f. rubocop/rubocop#7469
@sue445 sue445 mentioned this pull request Nov 27, 2019
Merged
pocke added a commit to pocke/mry that referenced this pull request Dec 5, 2019
@pocke
Support renaming "Whitelist"
c462f8f 
rubocop/rubocop#7469
@pocke pocke mentioned this pull request Dec 5, 2019
Merged
sihugh added a commit to alphagov/rubocop-govuk that referenced this pull request Dec 30, 2019
@sihugh
Fix AllowedMethod parameter name
d226799 
Resolves warning:

> Warning: Style/TrivialAccessors does not support AllowedMethod parameter.
>
> Supported parameters are:
>
>  - Enabled
>  - ExactNameMatch
>  - AllowPredicates
>  - AllowDSLWriters
>  - IgnoreClassMethods
>  - AllowedMethods

(Issue where bug introduced to Rubocop)[rubocop/rubocop#7469]
(Issue describing resolution)[rubocop/rubocop#7530]
@lewispb lewispb mentioned this pull request Jun 29, 2020
Merged
@koic koic mentioned this pull request Jun 1, 2021
Merged
@koic koic mentioned this pull request Aug 2, 2023
Closed
ydah pushed a commit to rubocop/rubocop-rspec_rails that referenced this pull request Mar 27, 2024
@bquorning
Change potentially offensive terminology
fbfbd50 
I think the few places we use the term "whitelist", we really don't need
to. Changing to use "allowed" etc. instead.

RuboCop did similar changes in
rubocop/rubocop#7469.
ydah pushed a commit to rubocop/rubocop-rspec_rails that referenced this pull request Mar 27, 2024
@bquorning
Change potentially offensive terminology
811998b 
I think the few places we use the term "whitelist", we really don't need
to. Changing to use "allowed" etc. instead.

RuboCop did similar changes in
rubocop/rubocop#7469.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbatsov bbatsov bbatsov left review comments

@movermeyer movermeyer movermeyer left review comments

@rrosenblum rrosenblum rrosenblum approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@koic @bbatsov @Drenmi @movermeyer @rrosenblum