Repository navigation
refactor: use maps.Copy for cleaner map handling - #9630
Conversation
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
CodeRabbit Configuration File (
|
| for peerPub, count := range counts { | ||
| a.peerCounts[peerPub] = count | ||
| } | ||
| maps.Copy(a.peerCounts, counts) |
There was a problem hiding this comment.
Have you looked into how such refactor could be applied to the whole codebase perhaps?
There was a problem hiding this comment.
Yes, I conducted a check, and the other several that seem to meet the criteria are all of slice type. Currently, I can find these few instances.
There was a problem hiding this comment.
I think there are a couple more. Perhaps would you mind examining the output of this program and fix those too for completeness? https://github.com/bhandras/mapsearch
There was a problem hiding this comment.
Great! The program you provided is very useful!
There was a problem hiding this comment.
There's still a few left afaict, could you please add those too?
| for peerPub, count := range counts { | ||
| a.peerCounts[peerPub] = count | ||
| } | ||
| maps.Copy(a.peerCounts, counts) |
There was a problem hiding this comment.
I think there are a couple more. Perhaps would you mind examining the output of this program and fix those too for completeness? https://github.com/bhandras/mapsearch
|
Also could you please make sure that you update the change log? |
Thank you for your guidance. I have replaced everything, and please review it again. However, I couldn't find the changelog file. |
The changelog is here: https://github.com/lightningnetwork/lnd/blob/master/docs/release-notes/release-notes-0.19.0.md |
Thanks. Add the changelog. Please review again. |
|
Sorry to bother you, but there's another thing I would like to consult you about: I proposed an optimization using max/min to simplify the code at #9632. Should I submit two PRs as I am now, or should I merge it into one with the current one? |
Signed-off-by: xinhangzhou <shuangcui@aliyun.com>
Change Description
Use the built-in methods from the standard library to simplify the code.
The same as btcsuite/btcd#2343
Steps to Test
Steps for reviewers to follow to test the change.
Pull Request Checklist
Testing
Code Style and Documentation
[skip ci]in the commit message for small changes.📝 Please see our Contribution Guidelines for further guidance.