Skip to content

Fix cargo dev new_lint for late lint passes #5930

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

Merged
merged 1 commit into from
Aug 23, 2020

Conversation

ebroto
Copy link
Member

@ebroto ebroto commented Aug 20, 2020

Since 30c046e, LateLintPass has only one lifetime parameter.

I'm not sure how to easily test this, I think adding this kind of tests would be probably part of #5394

changelog: none

@rust-highfive
Copy link

r? @matthiaskrgr

(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 Aug 20, 2020
@matthiaskrgr
Copy link
Member

I'm not sure how to easily test this

We could just run

cargo dev new_lint  --name new_early_pass --pass early
cargo dev new_lint  --name new_late_pass --pass late
cargo check

somewhere in our CI pipeline I guess?

@ebroto
Copy link
Member Author

ebroto commented Aug 20, 2020

Oh good point, I'll look into that.

I'm not familiar enough with the pipeline, but I guess that there should also be some cleanup in case more commits are pushed and the workspace is reused.

@matthiaskrgr
Copy link
Member

I think this can be added here https://github.com/rust-lang/rust-clippy/blob/master/.github/workflows/clippy.yml#L94

Something like that may do:

    - name: cargo-dev new early lint test 
      run: cargo dev new_lint --name new_early_pass --pass early

    - name: cargo-dev new late lint test 
      run: cargo dev new_lint --name new_late_pass --pass late

    - name: cargo-dev build check
      run: cargo check

    - name: revert cargo-dev changes
      run: git reset --hard HEAD

@ebroto
Copy link
Member Author

ebroto commented Aug 20, 2020

Will try this tomorrow ASAP :), thanks!

@matthiaskrgr matthiaskrgr added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Aug 21, 2020
@ebroto ebroto force-pushed the dev_newlint_fix_lt branch from 43cd551 to db6815b Compare August 23, 2020 07:41
@ebroto
Copy link
Member Author

ebroto commented Aug 23, 2020

Testing the failure case...

@ebroto ebroto force-pushed the dev_newlint_fix_lt branch from db6815b to 23aada5 Compare August 23, 2020 08:02
@ebroto ebroto force-pushed the dev_newlint_fix_lt branch from 23aada5 to 6dd65b8 Compare August 23, 2020 08:14
@ebroto
Copy link
Member Author

ebroto commented Aug 23, 2020

Testing the success case...

(Hmm I should have left the previous commit showing the failure for review)

@ebroto
Copy link
Member Author

ebroto commented Aug 23, 2020

@matthiaskrgr I think it should be ready to review now :)

Copy link
Member

@matthiaskrgr matthiaskrgr left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@matthiaskrgr
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 23, 2020

📌 Commit 6dd65b8 has been approved by matthiaskrgr

@bors
Copy link
Contributor

bors commented Aug 23, 2020

⌛ Testing commit 6dd65b8 with merge 2d86cc5...

@bors
Copy link
Contributor

bors commented Aug 23, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: matthiaskrgr
Pushing 2d86cc5 to master...

@bors bors merged commit 2d86cc5 into rust-lang:master Aug 23, 2020
@ebroto ebroto deleted the dev_newlint_fix_lt branch August 23, 2020 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants