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

using slices.Contains to simplify the code #7837

Open
dashangcun opened this issue Jan 10, 2025 · 4 comments · May be fixed by #7833
Open

using slices.Contains to simplify the code #7837

dashangcun opened this issue Jan 10, 2025 · 4 comments · May be fixed by #7833

Comments

@dashangcun
Copy link

dashangcun commented Jan 10, 2025

This is a new function added in the go1.21 standard library, which can make the code more concise and easy to read.

The specific code is similar to https://github.com/cosmos/ibc-go/pull/7833/files.

If this issue can be approved by the maintainer, I would be happy to find such optimizations for all go projects under the cosmos organization.

@dashangcun dashangcun linked a pull request Jan 10, 2025 that will close this issue
10 tasks
@gjermundgaraba
Copy link
Contributor

I think this would be nice to use. Have you had a chance to see approximately how many places this would be possible to use in ibc-go?

@dashangcun
Copy link
Author

I think this would be nice to use. Have you had a chance to see approximately how many places this would be possible to use in ibc-go?

Of course, I am happy to do it. After searching and analyzing, there is only one place in the ibc-go project, which is the PR I submitted. If possible, please reopen this PR for review.

But there are more than 10 places in the cosmos-sdk project. Do I need to submit an issue for modification? Or just submit a PR directly (the changes may be more)

@gjermundgaraba
Copy link
Contributor

I'll reopen the PR, thanks for looking into this 🙏

I don't run the SDK team, but I would open a new issue saying you can open a PR for this shortly if they want the changes :)

@dashangcun
Copy link
Author

I'll reopen the PR, thanks for looking into this 🙏

I don't run the SDK team, but I would open a new issue saying you can open a PR for this shortly if they want the changes :)

Thanks!

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 a pull request may close this issue.

2 participants