-
Notifications
You must be signed in to change notification settings - Fork 116
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: remove unused method FilterSolanaInboundEvents
#3440
Conversation
📝 WalkthroughWalkthroughThe pull request focuses on cleaning up and streamlining the Solana inbound event filtering mechanism in the ZetaChain project. The primary change involves removing the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
FilterSolanaInboundEvents
FilterSolanaInboundEvents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
changelog.md (1)
Line range hint
26-35
: Consider removing duplicate entries.The following entries appear in both v26.0.0 and Unreleased sections:
- PR refactor(pkg/chains): add Avalanche, Arbitrum and World Chain in chain info #3379 (add Avalanche, Arbitrum and World Chain in chain info)
- PR fix: remove minimum rent exempt check for SPL token withdrawals #3374 (remove minimum rent exempt check for SPL token withdrawals)
This duplication could cause confusion about which version introduced these changes.
Keep these entries only in the v26.0.0 section since they were released in that version.
🧹 Nitpick comments (2)
changelog.md (2)
22-22
: Maintain consistent indentation in changelog entries.The entry
* [3440](https://github.com/zeta-chain/node/pull/3440) - remove unused method 'FilterSolanaInboundEvents'
has inconsistent indentation compared to other entries in the same section.-* [3440](https://github.com/zeta-chain/node/pull/3440) - remove unused method `FilterSolanaInboundEvents` + * [3440](https://github.com/zeta-chain/node/pull/3440) - remove unused method `FilterSolanaInboundEvents`
Line range hint
37-43
: Fix inconsistent version header formatting.The version header has an extra "Unreleased" section that breaks the consistency of the changelog structure.
## v25.0.0 - -## Unreleased
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
changelog.md
(1 hunks)zetaclient/chains/solana/observer/inbound.go
(0 hunks)zetaclient/chains/solana/observer/inbound_test.go
(0 hunks)
💤 Files with no reviewable changes (2)
- zetaclient/chains/solana/observer/inbound.go
- zetaclient/chains/solana/observer/inbound_test.go
🔇 Additional comments (2)
changelog.md (2)
Line range hint
1-24
: LGTM! Well-structured changelog section for unreleased changes.The unreleased section follows the standard format with clear categorization of changes into Features, Refactor, and Fixes sections. Each entry includes a PR link and concise description.
Line range hint
1-1000
: Verify version ordering in changelog.The changelog shows version jumps (e.g., v26.0.0 to v24.0.0) and some versions appear out of order. This makes it difficult to track the chronological progression of changes.
Run this script to verify version ordering:
✅ Verification successful
Changelog version ordering is correct and follows semantic versioning
The changelog properly lists versions in descending order from v26.0.0 down to v16.0.0, following standard semantic versioning practices. The apparent version "jumps" are normal and represent major version increments. Each version is correctly positioned chronologically with newer versions appearing before older ones.
The script's error message was due to incorrect validation logic rather than any actual ordering issues in the changelog.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Extract and verify version numbers are in descending order # Extract version numbers and check order echo "Checking version order in changelog..." versions=$(grep -E '^## v[0-9]+\.[0-9]+\.[0-9]+' changelog.md | cut -d' ' -f2) echo "Found versions: $versions" # Convert versions to comparable numbers and check if sorted prev_ver=999999 for ver in $versions; do # Remove 'v' prefix and convert to comparable number num_ver=$(echo $ver | sed 's/v//' | awk -F. '{printf "%d%03d%03d", $1, $2, $3}') if [ $num_ver -gt $prev_ver ]; then echo "Version order issue: $ver appears after a lower version" exit 1 fi prev_ver=$num_ver doneLength of output: 921
FilterSolanaInboundEvents
FilterSolanaInboundEvents
Description
Remove unused
FilterSolanaInboundEvents
and the corresponding unit test purely for nil logger.How Has This Been Tested?
Summary by CodeRabbit
Refactor
FilterSolanaInboundEvents
Cleanup