Remove proc_macro from the tidy whitelist again #39247
Conversation
|
Adding items to the whitelist is usually not wanted, but in this case this was done to avoid a bug/missing feature, so I'd say it was legitimate. |
| @@ -164,13 +165,14 @@ pub fn check(path: &Path, bad: &mut bool) { | |||
| }); | |||
|
|
|||
| // FIXME get this whitelist empty. | |||
| // Especially, only remove things. | |||
There was a problem hiding this comment.
I don't think we need this comment (i.e. the FIXME is sufficient). @abonander and I found and modified tools/tidy/src/features.rs only after exhausting all other ways to fix the tidy error.
There was a problem hiding this comment.
Maybe something like "Adding to this list should be avoided if possible"
| @@ -71,6 +71,11 @@ fn filter_dirs(path: &Path) -> bool { | |||
| skip.iter().any(|p| path.ends_with(p)) | |||
| } | |||
|
|
|||
| fn walk_many(paths: &Vec<&Path>, skip: &mut FnMut(&Path) -> bool, f: &mut FnMut(&Path)) { | |||
There was a problem hiding this comment.
nit: paths: &[Path] would be nicer so that the call would be
walk_many(&[path.join("test/compile-fail"), path.join("test/compile-fail-fulldeps")], ...)There was a problem hiding this comment.
I've tried &[Path] originally as well, but it gave me an error that Path is not sized (doc says this too). And when I did &[&Path] it gave me a type mismatch, not being able to coerce the &Vec to the slice.
There was a problem hiding this comment.
The type would be &[PathBuf].
There was a problem hiding this comment.
Interesting, seems that &[&Path] works with &[&path.join .... I've changed it to use that notation.
| @@ -115,7 +115,8 @@ pub fn check(path: &Path, bad: &mut bool) { | |||
| } | |||
| }); | |||
|
|
|||
| super::walk(&path.join("test/compile-fail"), | |||
| super::walk_many(&vec![&path.join("test/compile-fail"), | |||
| &path.join("test/compile-fail-fulldeps")], | |||
| &mut |path| super::filter_dirs(path), | |||
There was a problem hiding this comment.
formatting nit: indent to the ( after walk_many
est31
force-pushed
the
master
branch
2 times, most recently
from
c204c3e
to
e3daab0
Compare
PR rust-lang#38842 has exposed that we were missing the src/test/compile-fail-fulldeps directory in the search for feature gate tests. Because the detection didn't work despite the effort to name the test appropriately and add a correct "// gate-test-proc_macro" comment, proc_macro was added to the whitelist. We fix this little weakness in the feature gate tidy check and add the src/test/compile-fail-fulldeps directory to the checked directories.
|
📌 Commit e3daab0 has been approved by |
|
⌛ Testing commit e3daab0 with merge 7821a9b... |
Remove proc_macro from the tidy whitelist again PR #38842 has exposed that we were missing the src/test/compile-fail-fulldeps directory in the search for feature gate tests. Because the detection didn't work despite the effort to name the test appropriately and add a correct "// gate-test-proc_macro" comment, proc_macro was added to the whitelist. We fix this little weakness in the feature gate tidy check and add the src/test/compile-fail-fulldeps directory to the checked directories. Part of issue #39059 .
|
☀️ Test successful - status-appveyor, status-travis |
PR #38842 has exposed that we were missing the src/test/compile-fail-fulldeps
directory in the search for feature gate tests. Because the detection didn't
work despite the effort to name the test appropriately and add a correct
"// gate-test-proc_macro" comment, proc_macro was added to the whitelist.
We fix this little weakness in the feature gate tidy check and add
the src/test/compile-fail-fulldeps directory to the checked directories.
Part of issue #39059 .