Skip to content

Let update_lints also generate the internal lints #5156

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
Feb 15, 2020

Conversation

flip1995
Copy link
Member

r? @phansch

changelog: none

@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 11, 2020
@flip1995 flip1995 mentioned this pull request Feb 11, 2020
6 tasks
@phansch phansch self-requested a review February 11, 2020 11:01
Copy link
Member

@phansch phansch left a comment

Choose a reason for hiding this comment

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

I believe when I first implemented this, I had concerns about internal lints showing up in user facing stuff. But since we have fn usable_lints now, I don't think that's a problem anymore, if we use it correctly.

LGTM 👍

@flip1995
Copy link
Member Author

I had concerns about internal lints showing up in user facing stuff.

With the explicit .chain(internal_lints) (like it is done somewhere with deprecated lints), this only affects the stuff in clippy_lints/src/lib.rs

@flip1995
Copy link
Member Author

r? @phansch

@phansch
Copy link
Member

phansch commented Feb 15, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Feb 15, 2020

📌 Commit 50a2f97 has been approved by phansch

@bors
Copy link
Contributor

bors commented Feb 15, 2020

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@flip1995
Copy link
Member Author

@bors treeclosed-

oops forgot to reopen the tree after doc update.

@bors
Copy link
Contributor

bors commented Feb 15, 2020

⌛ Testing commit 50a2f97 with merge 933a0a9...

bors added a commit that referenced this pull request Feb 15, 2020
Let update_lints also generate the internal lints

r? @phansch

changelog: none
@bors
Copy link
Contributor

bors commented Feb 15, 2020

☀️ Test successful - checks-travis, status-appveyor
Approved by: phansch
Pushing 933a0a9 to master...

@bors bors merged commit 50a2f97 into rust-lang:master Feb 15, 2020
@flip1995 flip1995 deleted the dev_add_internal branch February 15, 2020 11:01
bors added a commit that referenced this pull request Feb 19, 2020
…logiq,flip1995

I like to move it, move it

GHA now runs in the background for 6 days (#5088)

Since then ~~15~~ 19 PRs were successfully merged and Travis+Appveyor agreed on the status in every case. ([GitHub PR search query](https://github.com/rust-lang/rust-clippy/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Amerged+merged%3A%3E%3D2020-02-12T15%3A42%3A00+sort%3Aupdated-desc+NOT+%5Bgh-pages%5D+in%3Atitle))

Some PRs were:
- #5163
- #5170
- #5168
- #5173
- #5171
- #5156
- #4809
- #5177
- #5182
- #5183
- #5184
- #5185
- #5186
- #5181
- #5189

Bug with GHA:
- When a rustc PR gets merged between the `integration_build` and the `integration` job, the `integration` job will fail. This happened once in #5162, but not in the past 6 days. Even if it would happen every 4th PR we would save time, since splitting up the integration build and tests saves 5-7 minutes per run and a complete run takes 15-17 minutes
- Sometimes the MacOS build takes up to an hour to download the master toolchain. Until now, this happend 2 or 3 times and can be resolved by a `@bors r3try`+canceling the previous run (restarting single jobs is not supported yet)

## Before merging this, rust-lang/rust-central-station#578 has to get merged

This PR is for starting the discussion and to get consensus (@rust-lang/clippy) on a final move to GHA. If we're ready, I'll contact Pietro, to finalize the move.

changelog: Clippy completely runs on GHA now 🎉

---

BTW: The deployment already runs on GHA, instead of Travis.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants