-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix documentation. #125
base: main
Are you sure you want to change the base?
Fix documentation. #125
Conversation
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.
First commit looks good, have yet to have a closer look at the second one.
Could you provide a PR description and adjust the title to reflect that it doesn't include just a documentation fix? Also, is the second commit related or should it live in an entirely separate PR?
channelMonitorsSerialized: serializedChannelMonitors, | ||
networkGraph: NetworkGraphArgument.instance(networkGraph), | ||
filter: filter, | ||
params: cmcParams | ||
) | ||
|
||
let channelManager = channelManagerConstructor.channelManager |
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.
While we're here, should we mention the optional fields (in particular the scorer params) below. Users recently seemed unaware how to set them.
Eh, I'll split this PR up into two parts. When I pushed the second commit I thought the doc PR had already been merged, and didn't bother pushing to a new remote branch. It should be separate. |
cef957a
to
b0a7101
Compare
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.
LGTM, mod the optional comment above.
No description provided.