-
-
Notifications
You must be signed in to change notification settings - Fork 282
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
docs: revert docs link removal #6864
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #6864 +/- ##
============================================
+ Coverage 62.20% 62.76% +0.56%
============================================
Files 571 578 +7
Lines 60021 61273 +1252
Branches 1976 2115 +139
============================================
+ Hits 37334 38456 +1122
- Misses 22644 22779 +135
+ Partials 43 38 -5 |
Performance Report✔️ no performance regression detected 🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
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.
Noticed there are plenty of links in our repo itself which lead to a "Page Not Found" e.g. lodestar/packages/beacon-node/README.md Line 21 in 14855ea
need to make sure to update those if we restructure docs |
forgot to mention this one on the call, it would be nice to get this merged for 1.20 |
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.
Leaving this up to @philknows for final review & approval
@@ -1,5 +1,5 @@ | |||
--- | |||
title: Stake with a Validator Client | |||
title: Starting a Validator Client |
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.
need to come up with some better title, "Validator Client" is redundant here. I liked the previous "Configuration" more, or maybe "Usage" (not sure myself)
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.
For consistency with how we've named "Start a beacon node", I think we should just stick to "Starting a Validator Client." We can always follow up on a new PR in the future.
@@ -17,7 +17,7 @@ | |||
- :gear: Follow the installation method for [source install](https://chainsafe.github.io/lodestar/getting-started/installation/#build-from-source), [NPM install](https://chainsafe.github.io/lodestar/getting-started/installation/#install-from-npm-not-recommended), or [Docker install](https://chainsafe.github.io/lodestar/getting-started/installation/#docker-installation) to install Lodestar. Or use our [Lodestar Quickstart scripts](https://github.com/ChainSafe/lodestar-quickstart). | |||
- :books: Use [Lodestar libraries](https://chainsafe.github.io/lodestar/supporting-libraries/libraries/) in your next Ethereum Typescript project. | |||
- :globe_with_meridians: Run a beacon node on [mainnet or a public testnet](https://chainsafe.github.io/lodestar/getting-started/starting-a-node/). | |||
- :computer: Utilize the whole stack by [starting a local testnet](https://chainsafe.github.io/lodestar/advanced-topics/setting-up-a-testnet/). | |||
- :computer: Utilize the whole stack by [starting a local testnet](https://chainsafe.github.io/lodestar/contribution/advanced-topics/setting-up-a-testnet/). |
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.
We might wanna do some research if there is tooling that checks if all links work, we've broken so many links unnoticed before and it might be worth to set up some CI tooling for 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.
This is a good idea. For now, I've gone through the docs to fix broken links in eda50df
Approving everything up to my commit. Fixed some minor details and ready to go unless someone has strong opposition to this current structure. |
| Memory | 4GB RAM | 8GB RAM | | ||
| Storage | 20GB available space SSD | 100GB available space SSD | | ||
| Internet | Broadband connection | Broadband connection | | ||
| | Minimum | Recommended | |
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.
Noting that these new updated specifications are based on recent experiences with running Lodestar and not necessarily based on baseline benchmark data. For CPU minimums, I based it off https://ethereum.org/en/run-a-node/ where they recommend a baseline CPU benchmark score of 6667+ and I picked out CPUs around this benchmark for minimums.
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'll be fixing up specific and outdated content in a new PR. Will rename that readme title there 😅 |
🎉 This PR is included in v1.20.0 🎉 |
Motivation
Make sure all relevant files are available in docs. Also improve collapsed section.