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

lintcheck: fix table layout #13825

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

klensy
Copy link
Contributor

@klensy klensy commented Dec 14, 2024

Makes lintcheck output table looks nice. Before:

2024-12-12T14:00:30.9043123Z | Lint                                       | Added   | Removed | Changed |
2024-12-12T14:00:30.9044069Z | ------------------------------------------ | ------: | ------: | ------: |
2024-12-12T14:00:30.9049932Z | [`clippy::float_arithmetic`](#user-content-float-arithmetic)   |       0 |      10 |       1 |

After https://github.com/rust-lang/rust-clippy/actions/runs/12329727234/job/34414382066?pr=13825:

2024-12-14T12:21:42.0455818Z | Lint                                                         | Added | Removed | Changed |
2024-12-14T12:21:42.0456602Z |--------------------------------------------------------------|-------|---------|---------|
2024-12-14T12:21:42.0460656Z | [`clippy::float_arithmetic`](#user-content-float-arithmetic) | 0     | 10      | 1       |

Spotted at https://github.com/rust-lang/rust-clippy/actions/runs/12297760099/job/34320221594?pr=13820

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Dec 14, 2024

r? @Centri3

rustbot has assigned @Centri3.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 14, 2024
@klensy klensy changed the title [dont merge] lintcheck: fix table output [dont merge] lintcheck: fix table layout Dec 14, 2024
Comment on lines 286 to 277
| ------------------------------------------ | ------: | ------: | ------: |
| [`clippy::float_arithmetic`](#user-content-float-arithmetic) | 0 | 0 | 0 |";
let expected = "| Lint | Added | Removed | Changed |
|--------------------------------------------------------------|-------|---------|---------|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't added back that fancy ---:

Copy link
Member

Choose a reason for hiding this comment

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

Right alignment is pretty nice for number columns, the markdown is rendered on the summary page e.g. here at https://github.com/rust-lang/rust-clippy/actions/runs/12297760099?pr=13820

I believe the print in the run itself is more for debugging, personally I don't think it's worth adding a dependency for

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, added dep, yes, but lintcheck didn't distributed to users and even cached in CI.

@klensy klensy changed the title [dont merge] lintcheck: fix table layout lintcheck: fix table layout Dec 14, 2024
@Centri3
Copy link
Member

Centri3 commented Dec 14, 2024

r? Alexendoo

@rustbot rustbot assigned Alexendoo and unassigned Centri3 Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

4 participants