Skip to content

Address Clippy lints#392

Merged
BurntSushi merged 1 commit intoBurntSushi:masterfrom
thaliaarchi:clippy-fixes
Apr 5, 2025
Merged

Address Clippy lints#392
BurntSushi merged 1 commit intoBurntSushi:masterfrom
thaliaarchi:clippy-fixes

Conversation

@thaliaarchi
Copy link
Copy Markdown
Contributor

@thaliaarchi thaliaarchi commented Mar 18, 2025 •
edited
Loading

Address several small Clippy suggestions.

@BurntSushi
Copy link
Copy Markdown
Owner

BurntSushi commented Mar 18, 2025

Please break this into 3 separate PRs.

@thaliaarchi thaliaarchi changed the title Use #[non_exhaustive] and address Clippy lints Address Clippy lints Mar 18, 2025
@thaliaarchi
Copy link
Copy Markdown
Contributor Author

thaliaarchi commented Mar 18, 2025

I've split out #393 and #394.

BurntSushi
BurntSushi requested changes Mar 18, 2025
View reviewed changes
Copy link
Copy Markdown
Owner

@BurntSushi BurntSushi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

A lot of these are because of MSRV bumps, where code can be marginally simplified because of new Rust features/APIs. Those are generally fine.

But at least some of these are preference things that I don't agree with. This is why I don't have Clippy running on my repos, because there's too much that Clippy does by default that I don't agree with. I've long wanted to sit down and find the subset of lints I do agree with, but this is a lot of work to do. And then applying it to every other repo I have is just a ton of busy work. And honestly, most of the Clippy lints flagged in my code is more stylistic than it is catching bugs.

Comment thread csv-core/src/reader.rs Outdated
Comment thread src/byte_record.rs Outdated
Comment thread src/byte_record.rs Outdated
Comment thread src/serializer.rs Outdated
Comment thread src/tutorial.rs Outdated
Comment thread src/writer.rs Outdated
@thaliaarchi
Copy link
Copy Markdown
Contributor Author

thaliaarchi commented Mar 18, 2025

Sorry for all the nits. I probably should have kept this to just #[non_exhaustive].

@thaliaarchi
Copy link
Copy Markdown
Contributor Author

thaliaarchi commented Apr 5, 2025

Hey, friendly ping :). Since you already did a review and it's somewhat fresh, I just want to check in.

It seems like we've filtered out the objectionable bits, so it might be ready. I know it's an unimportant change, but I'd like to resolve this PR or even, since you don't usually use Clippy, just close it.

BurntSushi
BurntSushi approved these changes Apr 5, 2025
View reviewed changes
Copy link
Copy Markdown
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@BurntSushi BurntSushi merged commit da00088 into BurntSushi:master Apr 5, 2025
10 checks passed
@thaliaarchi thaliaarchi deleted the clippy-fixes branch April 5, 2025 18:50
@thaliaarchi thaliaarchi mentioned this pull request Apr 5, 2025
Closed
jqnatividad added a commit to dathere/rust-csv that referenced this pull request Jun 4, 2025
@jqnatividad
chore: sync with csv upstream
9253077 
so it aligns with BurntSushi#392
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BurntSushi BurntSushi BurntSushi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@thaliaarchi @BurntSushi