-
Notifications
You must be signed in to change notification settings - Fork 382
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
feat: overhaul lazy init in gnoland start
#1985
feat: overhaul lazy init in gnoland start
#1985
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1985 +/- ##
==========================================
- Coverage 49.08% 45.87% -3.21%
==========================================
Files 576 521 -55
Lines 77806 72445 -5361
==========================================
- Hits 38191 33236 -4955
+ Misses 36521 36466 -55
+ Partials 3094 2743 -351
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Before we continue, I'd like to clarify a few points.
Could you please specify the expected behavior and update docs/
?
How does gnoland start
function in different contexts?
@moul
It is however completely backwards compatible with users that still use only |
Any addition of a subcommand in an official tool should include documentation updates in the same pull request.
OK, thx.
Ok, great. Please wait for a discussion before starting #1886. |
Could you place "init" before "start" in the Ensure that the documentation begins with a suggested flow, starting with "init" and then "start," rather than delving into technical details and parameters right away. Your documentation is well-written, but it focuses on advanced scenarios. We should prioritize having essential documentation come first. |
I was under the impression that ffcli sorts the subcommands alphabetically for whatever reason (regardless of how we add subcommands), but that thankfully isn't the case, so I've swapped the order:
Are you suggesting I add something additional to the We will fill out the |
I recommend focusing on documenting workflows. We should ensure including Consider renaming your page to "Start a Gnoland Node" for clarity. Begin with a simple case and progress to advanced cases later, if needed. |
Just took code overview, seems nice ( I'm testing with actual binary now ). Found one small minor thing, which is |
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 took a look at the tutorial and ran through commands locally, everything seems to be working and is understandable.
This tutorial is aimed at people who want to run validators and connect to a network.
We are moving away from having a local node setup tutorial in the Getting Started section, as this section should appeal to people interested in Gno development (for which they will use gno
, gnokey
, and gnodev
), instead of a full local node setup. This PR, which shows how the Getting Started section will look like, has been sitting for some time now awaiting to be merged.
I will approve of this PR, but can you please make a new section in the docs, called “Gno Infra” where this tutorial can live along side other infra tutorials (such as the ones upcoming from OnBloc, as well as the tx-indexer, tx-archive, etc.)? Also, please add the new section to sidebars.js
file in the docs repo. 🙏
gnoland init
gnoland start
Description
This PR started as an implementation of the
gnoland init
command, that initializes the node configuration and secrets, as part of #1885.However, throughout the lifetime of this PR, discussions with @moul have shaped the PR to take a different approach:
gnoland init
, because it's an alias, does not warrant its own subcommand (it's a combo ofgnoland config init
andgnoland secrets init
gnoland start
should have a much more clear lazy init flow, and it should be optional (I added this with the--lazy
flag). Here is an example of the flow:gnoland init
is adapted to use thegnoland config
and thegnoland secrets
command suites (this was easy to do, they are an alias)gnoland start
lazy init regenerated everything that was missing and skipped everything that was present. Partial initialization means that missing secrets are regenerated, and existing ones are skippedgnoland start
, with more to be pruned in future PRs. These were legacy / leftover, as we've added better support for them in the meantimeAs a consequence of these discussions, we have decided to table lazy init removal for the future -- #1886
Closes #1885
Thank you @albttx, @r3v4s, and the team for discussions that led us to finalize this bigger effort of node init flows 🙏
Contributors' checklist...
BREAKING CHANGE: xxx
message was included in the description