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

Don't rustfmt check the vendor directory. #69047

Merged
merged 1 commit into from
Feb 11, 2020
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Feb 11, 2020

I need to be able to run x.py tidy to do license checks (which requires vendored dependencies). However, when vendoring is enabled, it wants to rustfmt check the entire vendor directory, which doesn't work.

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(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 Feb 11, 2020
@ehuss
Copy link
Contributor Author

ehuss commented Feb 11, 2020

I would prefer if the rustfmt check was restricted to the src directory and ignored all other top-level directories. But I don't know if it was written the way it was for a reason.

@@ -7,6 +7,7 @@ merge_derives = false
# tidy only checks files which are not ignored, each entry follows gitignore style
ignore = [
"build",
"/vendor/",
Copy link
Contributor

Choose a reason for hiding this comment

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

double check: should there be a leading / here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, it's a gitignore pattern:

  • foo — any file or directory named foo anywhere in the repo.
  • foo/ — any directory named foo anywhere in the repo.
  • /foo — a file or directory named foo at the root of the repo.
  • /foo/ — a directory named foo at the root of the repo.

The vendor directory is created in the root.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok; so not the root of the filesystem :D

@Centril
Copy link
Contributor

Centril commented Feb 11, 2020

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 11, 2020

📌 Commit 6575abc has been approved by Centril

@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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 11, 2020
@Centril
Copy link
Contributor

Centril commented Feb 11, 2020

r? @Centril

@Mark-Simulacrum
Copy link
Member

We can likely revise to ignore all but src; I wrote it this way to avoid accidentally not checking some directory, but there's no expectation in my mind that we would add such a directory.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 11, 2020
Don't rustfmt check the vendor directory.

I need to be able to run `x.py tidy` to do license checks (which requires vendored dependencies).  However, when vendoring is enabled, it wants to rustfmt check the entire vendor directory, which doesn't work.
bors added a commit that referenced this pull request Feb 11, 2020
Rollup of 8 pull requests

Successful merges:

 - #66498 (Remove unused feature gates)
 - #68816 (Tweak borrow error on `FnMut` when `Fn` is expected)
 - #68824 (Enable Control Flow Guard in rustbuild)
 - #69022 (traits: preallocate 2 Vecs of known initial size)
 - #69031 (Use `dyn Trait` more in tests)
 - #69044 (Don't run coherence twice for future-compat lints)
 - #69047 (Don't rustfmt check the vendor directory.)
 - #69055 (Clean up E0307 explanation)

Failed merges:

r? @ghost
@bors bors merged commit 6575abc into rust-lang:master Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants