Skip to content

Conversation

ysangkok
Copy link
Contributor

@ysangkok ysangkok commented Jan 8, 2021

Feedback appreciated.

Copy link
Owner

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I think this is useful! I think each section should contain:

  1. A high-level, noob-friendly description of what the feature achieves (not done for all features yet)
  2. Historical details about activation in main implementations (which you already did)

I'd also like to see the following features mentioned:

  • upfront_shutdown_script: even though eclair still doesn't support it and it's not commonly used, this can be a useful feature for some users
  • gossip_queries / gossip_queries_ex (I think we can have a single paragraph for both, as both should generally be enabled together)
  • It's probably a bit too early to mention anchor outputs

@ysangkok
Copy link
Contributor Author

@t-bast I pushed a commit that addresses all your comments.

Copy link
Owner

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, a last round of comments and we should be good to go.

@ysangkok
Copy link
Contributor Author

@t-bast Ok, I have applied the suggestions and squashed.

Copy link
Owner

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@t-bast t-bast merged commit 186e319 into t-bast:master Jan 14, 2021
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