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

refactor: use strings.EqualFold #482

Merged
merged 2 commits into from
Jan 16, 2025
Merged

refactor: use strings.EqualFold #482

merged 2 commits into from
Jan 16, 2025

Conversation

zhuhaicity
Copy link
Contributor

@zhuhaicity zhuhaicity commented Jan 12, 2025

Summary

strings.EqualFold has no memory overhead and has better performance than strings.ToLower.

This is a performance test:

package bench

import (
	"strings"
	"testing"
)

func BenchmarkToLower(b *testing.B) {

	str1 := "Windows"
	str2 := "windows"

	for i := 0; i < b.N; i++ {
		if strings.ToLower(str1) == strings.ToLower(str2) {
		}
	}
}

func BenchmarkEqualFold(b *testing.B) {

	str1 := "Windows"
	str2 := "windows"

	for i := 0; i < b.N; i++ {
		if strings.EqualFold(str1, str2) {
		}
	}
}

The result:

goos: darwin
goarch: arm64
BenchmarkToLower-8      31404808                36.99 ns/op            8 B/op          1 allocs/op
BenchmarkEqualFold-8    194780793                5.989 ns/op           0 B/op          0 allocs/op
PASS

Task/Issue reference

Closes: add_link_here

Test plan

Please describe how reviewers can test the changes in this pull request. If the test plan is challenging to explain in text alone, a short video demonstration may be included.

Details (optional)

Add any additional details that will help to review this pull request.

Related issues or PRs (optional)

Add any related issues or PRs.

@zhuhaicity zhuhaicity requested a review from a team as a code owner January 12, 2025 07:48
Copy link

cla-bot bot commented Jan 12, 2025

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @zhuhaicity on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again.

Signed-off-by: zhuhaicity <zhuhai@52it.net>
Copy link

cla-bot bot commented Jan 12, 2025

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @zhuhaicity on file. In order for us to review and merge your code, please open a new pull request and sign the Contributor License Agreement following the instructions in our contributing guide. Once the pull request is merged, ping a maintainer who will summon me again.

@cla-bot cla-bot bot added the cla-signed label Jan 13, 2025
Copy link
Contributor

@bgins bgins left a comment

Choose a reason for hiding this comment

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

Excellent, thanks @zhuhaicity! Much faster. 😄

We are holding off on merging PRs until after tomorrow's release, but I'll this merged right after.

Question for you. Would you like to receive Lilybit rewards for your contribution? We have an open source program to reward contributors when we launch to mainnet.

@zhuhaicity
Copy link
Contributor Author

Excellent, thanks @zhuhaicity! Much faster. 😄

We are holding off on merging PRs until after tomorrow's release, but I'll this merged right after.

Question for you. Would you like to receive Lilybit rewards for your contribution? We have an open source program to reward contributors when we launch to mainnet.

Ha ha, thank you so much. It’s my honor.

If there’s a reward, that would be great! Do I need to provide anything?

@bgins
Copy link
Contributor

bgins commented Jan 14, 2025

If there’s a reward, that would be great! Do I need to provide anything?

In general, we are tracking contributions that have been taken up by community through issues. I've opened an issue to track your contribution here: #485

It would be helpful to know your Discord username so we can contact you when we distribute rewards. Are you in the Lilypad Discord? If not, would you be up for joining? https://discord.gg/DHMgfVSD

@bgins
Copy link
Contributor

bgins commented Jan 14, 2025

@zhuhaicity actually, there is one thing you could do. Could you comment on the issue I made for this contribution? #485

A simple comment like "I have implemented this" would be fine. It's just a formality so I can assign you to the issue in GitHub for our tracking.

@bgins
Copy link
Contributor

bgins commented Jan 14, 2025

We've delayed our release, so might be a couple more days before we get this merged.

@zhuhaicity
Copy link
Contributor Author

If there’s a reward, that would be great! Do I need to provide anything?

In general, we are tracking contributions that have been taken up by community through issues. I've opened an issue to track your contribution here: #485

It would be helpful to know your Discord username so we can contact you when we distribute rewards. Are you in the Lilypad Discord? If not, would you be up for joining? https://discord.gg/DHMgfVSD

Thank you very much!

I have joined the discord group, my id is bitcatty.

@zhuhaicity
Copy link
Contributor Author

We've delayed our release, so might be a couple more days before we get this merged.

Already commented. No rush, thank you very much!

@bgins
Copy link
Contributor

bgins commented Jan 15, 2025

We've delayed our release, so might be a couple more days before we get this merged.

Already commented. No rush, thank you very much!

Alright great! Assigned you to the issue, so we should be good to go. 😃

@bgins bgins merged commit 2d85b22 into Lilypad-Tech:main Jan 16, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants