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

Avoid "whitelist" #74127

Merged
merged 1 commit into from
Jul 11, 2020
Merged

Avoid "whitelist" #74127

merged 1 commit into from
Jul 11, 2020

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Jul 7, 2020

Other terms are more inclusive and precise.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2020
@ratijas

This comment has been minimized.

@ratijas

This comment has been minimized.

@tamird
Copy link
Contributor Author

tamird commented Jul 8, 2020

The "third party config variable" was a comment.

cc @steveklabnik and @nikomatsakis, I guess. The reaction to this PR demonstrates the reason this sort of change should be made sooner rather than later.

@steveklabnik
Copy link
Member

r? @rust-lang/compiler

@rust-lang rust-lang locked as too heated and limited conversation to collaborators Jul 8, 2020
@BurntSushi
Copy link
Member

BurntSushi commented Jul 8, 2020

Moderation note: Some of the comments already posted on this PR are completely inappropriate. While dissent is always encouraged, it must be provided in a constructive manner. Mixing it with personal attacks is not constructive and it will not be tolerated in official Rust community spaces.

Given the nature of this PR, I am locking this PR with the intent that relevant stakeholders are given time to decide how they want to handle this first. It looks like that's probably @rust-lang/compiler.

src/librustc_codegen_ssa/back/linker.rs Outdated Show resolved Hide resolved
src/librustc_codegen_llvm/attributes.rs Outdated Show resolved Hide resolved
src/tools/clippy/clippy_lints/src/eq_op.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Left a few nits, but I am in favor of removing the term "whitelist" and moving to "allowlist" or other more precise terminology (big 👍 to "AssumeUsed" in particular). This doesn't look like it affects any "user visible" flags or anything like that, it's purely internal? (I would want to have some kind of "deprecation" for the old flags if there were any.)

src/librustc_codegen_llvm/attributes.rs Outdated Show resolved Hide resolved
src/librustc_middle/lint.rs Outdated Show resolved Hide resolved
@tamird tamird force-pushed the allowlist branch 2 times, most recently from 99880e8 to 23f8cac Compare July 8, 2020 13:53
src/librustc_codegen_llvm/llvm_util.rs Outdated Show resolved Hide resolved
src/librustc_codegen_llvm/llvm_util.rs Outdated Show resolved Hide resolved
@tamird tamird force-pushed the allowlist branch 2 times, most recently from 2b8657b to 8081312 Compare July 8, 2020 15:02
src/bootstrap/doc.rs Outdated Show resolved Hide resolved
src/librustc_expand/base.rs Outdated Show resolved Hide resolved
@oli-obk
Copy link
Contributor

oli-obk commented Jul 10, 2020

@bors r+ p=1 (bitrottyish)

@bors
Copy link
Contributor

bors commented Jul 10, 2020

📌 Commit 62cf767 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 10, 2020
Avoid "whitelist"

Other terms are more inclusive and precise.
@bors
Copy link
Contributor

bors commented Jul 11, 2020

⌛ Testing commit 62cf767 with merge 2bba01db2eec0bab687b341af01c99cb1e7341fd...

Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 11, 2020
Avoid "whitelist"

Other terms are more inclusive and precise.
@Manishearth
Copy link
Member

@bors retry yield

Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 11, 2020
Avoid "whitelist"

Other terms are more inclusive and precise.
@tesuji

This comment has been minimized.

whitelist = sys.argv[1:]
if whitelist:
tests = [test for test in tests if test in whitelist]
listed = sys.argv[1:]
Copy link
Contributor

Choose a reason for hiding this comment

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

listed sound like a bool to me. Could we change it to args or arguments ?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea listed is abit weird word here args seems reasonable.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 11, 2020
…arth

Rollup of 19 pull requests

Successful merges:

 - rust-lang#71322 (Accept tuple.0.0 as tuple indexing (take 2))
 - rust-lang#72303 (Add core::future::{poll_fn, PollFn})
 - rust-lang#73862 (Stabilize casts and coercions to `&[T]` in const fn)
 - rust-lang#73887 (stabilize const mem::forget)
 - rust-lang#73989 (adjust ub-enum test to be endianess-independent)
 - rust-lang#74045 (Explain effects of debugging options from config.toml)
 - rust-lang#74076 (Add `read_exact_at` and `write_all_at` to WASI's `FileExt`)
 - rust-lang#74099 (Add VecDeque::range* methods)
 - rust-lang#74100 (Use str::strip* in bootstrap)
 - rust-lang#74103 (Only add CFGuard on `windows-msvc` targets)
 - rust-lang#74109 (Only allow `repr(i128/u128)` on enum)
 - rust-lang#74122 (Start-up clean-up)
 - rust-lang#74125 (Correctly mark the ending span of a match arm)
 - rust-lang#74127 (Avoid "whitelist")
 - rust-lang#74129 (:arrow_up: rust-analyzer)
 - rust-lang#74135 (Update books)
 - rust-lang#74145 (Update rust-installer to latest version)
 - rust-lang#74161 (Fix  disabled dockerfiles)
 - rust-lang#74162 (take self by value in ToPredicate)

Failed merges:

r? @ghost
@SomeRandomGuy81

This comment has been minimized.

@bors bors merged commit d2f8c30 into rust-lang:master Jul 11, 2020
@tamird tamird deleted the allowlist branch July 11, 2020 10:28
@BurntSushi
Copy link
Member

Now that the PR has been merged and this PR seems to have attracted trolls, I'm going to lock it for good. As Niko said above, name improvements can be iterated on in follow up PRs.

Thank you to everyone who participated in this PR constructively!

@rust-lang rust-lang locked as resolved and limited conversation to collaborators Jul 11, 2020
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.