Skip to content

refactor: use slices.Equal to simplify code #7678

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

Open
wants to merge 1 commit into
base: v3.35
Choose a base branch
from

Conversation

minxinyi
Copy link

Summary

In the Go 1.21 standard library, a new function has been introduced that enhances code conciseness and readability. It can be find here.

...

Changes

  • ...
  • ...

Testing

Steps

...

Results
Regressions

...

Notes for Reviewers

...

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • Testing: The steps/process to test this feature are clearly explained including testing for regressions.
  • Infrastructure: If infrastructural changes (e.g., new RPC, configuration) are needed, a separate issue is created in the infrastructural repositories.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@minxinyi minxinyi requested a review from a team as a code owner July 23, 2025 09:52
@minxinyi minxinyi requested a review from nicholaspcr July 23, 2025 09:52
@nicholaspcr
Copy link
Contributor

Thanks for your contribution, @minxinyi! You're right, using slices.Equal is a much better approach than the custom function.

I did a quick search and it looks like the old EqualChMasks function is still being used in a few other places in the band package. Would you be willing to update those as well? That would make this contribution even more complete. If you do a search for EqualChMasks in the codebase, you should be able to find the other locations easily.

Once those are updated, I'd be happy to approve and merge this PR.

@minxinyi minxinyi requested a review from a team as a code owner July 25, 2025 18:37
@minxinyi minxinyi requested a review from halimi July 25, 2025 18:37
@minxinyi
Copy link
Author

Thanks for your contribution, @minxinyi! You're right, using slices.Equal is a much better approach than the custom function.

I did a quick search and it looks like the old EqualChMasks function is still being used in a few other places in the band package. Would you be willing to update those as well? That would make this contribution even more complete. If you do a search for EqualChMasks in the codebase, you should be able to find the other locations easily.

Once those are updated, I'd be happy to approve and merge this PR.

Thanks for your reply.

Modified. Please review it when you have time.

@nicholaspcr
Copy link
Contributor

Hi @minxinyi. I investigated the CI failure and it was caused by a pre-existing issue in v3.35 that is now resolved. Please rebase your branch against the latest v3.35 to bring in the fix. Thanks for your contribution

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