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.

Sign up for GitHub

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

add completions for clippy lint in attributes #6109

Merged
merged 1 commit into from
Oct 20, 2020
Merged

add completions for clippy lint in attributes #6109

merged 1 commit into from
Oct 20, 2020

Conversation

bnjjj
Copy link
Contributor

@bnjjj bnjjj commented Oct 1, 2020

No description provided.

@Veykril
Copy link
Member

Veykril commented Oct 1, 2020

Reqwest seems like quite a big dependency for just simple get requests, I think attohttpc with the json feature would be a better choice?

@bnjjj
Copy link
Contributor Author

bnjjj commented Oct 1, 2020

Reqwest seems like quite a big dependency for just simple get requests, I think attohttpc with the json feature would be a better choice?

done @Veykril thanks for you suggestion

@bnjjj bnjjj force-pushed the master branch 2 times, most recently from 2173416 to d6cd46d Compare October 1, 2020 14:11
@lnicola
Copy link
Member

lnicola commented Oct 6, 2020

Maybe clippy could add a -W help option to display a list of warning names (similar to rustc -W help, but more machine-parsable)?

@bnjjj
Copy link
Contributor Author

bnjjj commented Oct 6, 2020

@lnicola Yes indeed it could be a good idea. I add this in my todo list. And I will make a new PR with these changes if it's good for you to iterate ?

@lnicola
Copy link
Member

lnicola commented Oct 6, 2020

CC rust-lang/rust-clippy#5385, see my last comment. It's already available, but doesn't work for us at the moment. I think we could live with the codegen approach for a while.

@bnjjj
Copy link
Contributor Author

bnjjj commented Oct 6, 2020

Let me clarify one thing, I don't fetch this url http://rust-lang.github.io/rust-clippy/master/index.html as you mentioned in the clippy issue but the json located here http://rust-lang.github.io/rust-clippy/master/lints.json which is far better than scrapping an html page :p

@bnjjj bnjjj requested a review from flodiebold October 6, 2020 12:31
@matklad
Copy link
Member

matklad commented Oct 6, 2020

Hm, I'd rather not add any networing, even to the build

@kiljacken
Copy link
Contributor

Hm, I'd rather not add any networing, even to the build

We're already doing the same sort of thing for rustc features, aren't we?

@matklad
Copy link
Member

matklad commented Oct 6, 2020

We are relying on external process for that, this is fine. I'd rather avoid adding more crates to our crate graph, unless we absolutely need them. HTTP crate feels like something we absolutely don't need :D

@bnjjj
Copy link
Contributor Author

bnjjj commented Oct 7, 2020

Maybe I can use curl to download the json as you do a git clone to codegen features

@bnjjj
Copy link
Contributor Author

bnjjj commented Oct 8, 2020

updated with network crate deleted

@bnjjj
Copy link
Contributor Author

bnjjj commented Oct 14, 2020

up @matklad if you can give me your thoughts :)

@lnicola
Copy link
Member

lnicola commented Oct 14, 2020

See also rust-lang/rust#77671.

matklad
matklad reviewed Oct 14, 2020
View reviewed changes
xtask/Cargo.toml Outdated Show resolved Hide resolved
matklad
matklad reviewed Oct 15, 2020
View reviewed changes
Copy link
Member

@matklad matklad left a comment

Choose a reason for hiding this comment

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

Overall approach looks good now, left some style comments.

xtask/tests/tidy.rs Outdated Show resolved Hide resolved
xtask/src/codegen/gen_lint_completions.rs Outdated Show resolved Hide resolved
xtask/src/codegen/gen_lint_completions.rs Outdated Show resolved Hide resolved
xtask/src/codegen/gen_lint_completions.rs Show resolved Hide resolved
@bnjjj
Copy link
Contributor Author

bnjjj commented Oct 16, 2020

@matklad updated with your suggests :) thanks for your review

@matklad
Copy link
Member

matklad commented Oct 20, 2020

lgmt with rebase and one last nit fixed

bors d+

@bors
Copy link
Contributor

bors bot commented Oct 20, 2020

✌️ bnjjj can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@bnjjj bnjjj force-pushed the master branch 2 times, most recently from 2e764b3 to 5ba8f75 Compare October 20, 2020 19:19
@bnjjj
add completions for clippy lint in attributes
aa031e9 
Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
@bnjjj
Copy link
Contributor Author

bnjjj commented Oct 20, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 20, 2020

Build succeeded:

@bors bors bot merged commit 08823c8 into rust-lang:master Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lnicola lnicola lnicola left review comments

@matklad matklad matklad left review comments

@SomeoneToIgnore SomeoneToIgnore SomeoneToIgnore left review comments

@est31 est31 est31 left review comments

@flodiebold flodiebold Awaiting requested review from flodiebold

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

Successfully merging this pull request may close these issues.
8 participants
@bnjjj @Veykril @lnicola @matklad @kiljacken @flodiebold @SomeoneToIgnore @est31