Skip to content

Lint cleanup#127

Merged
willbush merged 11 commits intomainfrom
lint-cleanup
Nov 11, 2024
Merged

Lint cleanup#127
willbush merged 11 commits intomainfrom
lint-cleanup

Conversation

@willbush
Copy link
Copy Markdown
Member

@willbush willbush commented Nov 8, 2024

Based on #124, but this time fewer pedantic lint fixes and cleaner git history.

@willbush
Copy link
Copy Markdown
Member Author

willbush commented Nov 8, 2024

@Ben-PH feel free to review too! (Github won't let me add you as a reviewer)

Copy link
Copy Markdown
Contributor

@Ben-PH Ben-PH left a comment

Choose a reason for hiding this comment

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

I like it.

Minor, inconsequential nit, but the map_or_else functions might be able to use Path::display (similar to "instead of |s| s.to_string(), use str::to_string"), but you do you.

@willbush
Copy link
Copy Markdown
Member Author

willbush commented Nov 8, 2024

Minor, inconsequential nit, but the map_or_else functions might be able to use Path::display (similar to "instead of |s| s.to_string(), use str::to_string"), but you do you.

Not seeing a way to do that without dropping the ./ appended to the beginning of the path.

@Ben-PH
Copy link
Copy Markdown
Contributor

Ben-PH commented Nov 8, 2024

Missed the ./ part. My mistake.

Copy link
Copy Markdown
Member

@infinisil infinisil 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!

@willbush willbush merged commit 54a1aec into main Nov 11, 2024
@willbush willbush deleted the lint-cleanup branch November 11, 2024 15:26
@mdaniels5757 mdaniels5757 mentioned this pull request Mar 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants