-
Notifications
You must be signed in to change notification settings - Fork 2k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Default values for updateChannelPolicyCommand #8673
base: master
Are you sure you want to change the base?
Conversation
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Thanks for the PR. I have to say, you're quite brave to tackle this issue... Many have tried before 馃槄 To avoid a lot of back-and-forth while reviewing, please make sure you have read all previous discussions in #6762 and understand the nuance of what a default value is, where it's shown, how it's used and what user expectations (and the server behavior) are around those flags. Also, please make sure commits are logical units that tell the reviewer what is happening, why and in what order. So multiple edits of the same line should usually be squashed into a single commit. |
I've studied the previous attempts and unfortunately I think there is going to be some minor back and forth. I added a comment here #6762 (comment) Good catch on the commits, I'll squash it into a single commit |
09865a5
to
45284af
Compare
45284af
to
191ee20
Compare
Change Description
ctx.IsSet()
will send 0 to the RPCfixes #1523
Steps to Test
Run updateChannelPolicyCommand command with -h and see defaults.
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.