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

use cow_to_ascii_lowercase instead #762

Closed
wants to merge 2 commits into from
Closed

Conversation

heygsc
Copy link
Contributor

@heygsc heygsc commented Jan 12, 2025

part of #761

https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods

If this PR can be merged, I will continue to use more cow methods in clippy.

@heygsc heygsc changed the title use cow_to_ascii_lowercase instead use cow_to_ascii_lowercase instead Jan 12, 2025
@heygsc
Copy link
Contributor Author

heygsc commented Jan 12, 2025

By the way, running clippy in make fix-format may cause some code to automatically change, which I did not submit because the clippy in make check-format allows it to pass.

These are not related to the methods I added, they already existed before. It seems that they don't need to be changed anymore?

@jaytaph
Copy link
Member

jaytaph commented Jan 12, 2025

By the way, running clippy in make fix-format may cause some code to automatically change, which I did not submit because the clippy in make check-format allows it to pass.

This is good. The PR should only reflect your changes, not unrelated format changes. I'm not sure why this is happening though.. The CI should automatically reject clippy issues that are not resolved, so everybody should have the same formatting. I've noticed this happens on occasion with other PR's, so it might be a good idea to look into this issue as well (later)..

@jaytaph
Copy link
Member

jaytaph commented Jan 12, 2025

Looks good.. Thanks @heygsc for the PR!

@jaytaph
Copy link
Member

jaytaph commented Jan 12, 2025

Ah, we found a small issue concerning your git commits. They need to be signed commits. For more info about signed commits see here: https://docs.github.com/authentication/managing-commit-signature-verification/about-commit-signature-verification

You should be able to either rebase your PR with a signed commit, or reopen a new PR with signed commits so we can merge.

@heygsc
Copy link
Contributor Author

heygsc commented Jan 12, 2025

Okay, I did neglect to carefully read the contribution document. I only realized it before modifying the AUTHORS, so the second commit has a gpg sign.

I will open a new PR later.

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.

2 participants