-
Notifications
You must be signed in to change notification settings - Fork 406
Remove Slack link and other updates to README
#1628
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
Remove Slack link and other updates to README
#1628
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1628 +/- ##
==========================================
- Coverage 90.80% 90.78% -0.03%
==========================================
Files 80 80
Lines 44654 44654
Branches 44654 44654
==========================================
- Hits 40550 40539 -11
- Misses 4104 4115 +11
Continue to review full report at Codecov.
|
README.md
Outdated
6. [lightning-persister](./lightning-persister) | ||
Utilities to manage Rust-Lightning channel data persistence and retrieval. | ||
Implements utilities to manage Rust-Lightning channel data persistence and retrieval. |
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.
Implements utilities to manage Rust-Lightning channel data persistence and retrieval. | |
Implements utilities to manage rust-lightning channel data persistence and retrieval. |
Repo name should probably be lowercase for consistency too (unless start of a sentence)? Here and elsewhere there's a mix of "Rust-Lightning" and "rust-lightning"
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.
Mh, I'm not sure if there is consensus on this. I'd say if we see Rust-Lightning as a thing that is more than the main crates itself, it probably should be capitalized. However, I'd argue rust-lightning
is just the Rust library and the 'more' part would be LDK and therefore agree to keep it consistently lower case. So I now replaced Rust-Lightning with rust-lightning
in most places, except for headings and the tag line.
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.
Ok, yeah that makes sense. I guess Rust-Lightning was in its previous life :)
README.md
Outdated
@@ -1,4 +1,4 @@ | |||
`rust-lightning` |
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.
Is there a reason for reverting this?
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.
Yes, I think rust-lightning
in headings and/or the tag line looks odd and decided to revert to title case for these, i.e., Rust-Lightning. Happy to always use rust-lightning
if there is a clear preference for that though.
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.
I prefer keeping it consistent, no big deal though.
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
I think feel free to squash. |
cde288f
to
cf7655a
Compare
Squashed |
This started off as me simply wanting to remove the deprecated Slack link, but I couldn't keep myself from making at least some additional changes while touching the README.
I think it could be updated quite a bit more, but these changes are maybe a first step.