docs: Incorporate BigLep documentation comments from PR #16 #33
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ical sorting - Rename ADVANCED_README.md to README_ADVANCED.md using git mv for clear diff tracking - Update reference in README.md to point to new filename - Addresses BigLep's comment about naming consistency
- Add link from SP nodes control to Configuration System section in README_ADVANCED.md - Add x86 architecture requirement note to System Requirements table - Addresses BigLep's comments about missing links and architecture limitation
- Add note that build must be run after init - Add note that start should be run after build of lotus and curio - Addresses BigLep's comment about missing command order information
- Explain that defaults are defined in code (src/config.rs Config::default()) - Document how init writes defaults to config.toml - Explain how to update defaults with new versions using init --force - Addresses BigLep's question about where defaults are defined and how they get updated
- Remove second duplicate Manual Cleanup section - Keep the first one which has more comprehensive content - Addresses BigLep's comment about duplicate sections
- Change Run ID format from YYmmmDD-HHMM to YYYY-MM-DDTHH:MM (ISO8601) - Update code in src/run_id/mod.rs to use new format - Update test regex to match new format - Update all documentation examples to use new format (2026-01-02T14:30_ZanyPip) - Addresses BigLep's comment about date format standardization for natural sorting and clarity
- Add definition of 'step' as discrete unit of work in cluster startup - Link to Detailed Start Sequence section for complete list - Reorganize Step Context section for better clarity - Addresses BigLep's comment about defining 'step' when first mentioned
- Link to src/commands/start/ directory for context key implementations - Add example link to usdfc_deploy/prerequisites.rs showing context.get() usage - Clarify that context keys are string literals, not constants - Addresses BigLep's comment about linking to definitive list in code
- Change plain text 'Portainer' to [Portainer](https://docs.docksal.io/use-cases/portainer/) - Addresses BigLep's comment about proper link formatting
- Clarify that Yugabyte is shared by all Curio SPs - Update port descriptions from 'Dynamic' to 'Dynamic from range' for all Curio containers - Addresses BigLep's comments about container descriptions
- Simplify diagram to show single representative instance with curio-n/yugabyte-n notation - Correct Yugabyte architecture: each SP has its own Yugabyte instance, not shared - Update container architecture table to show yugabyte-1 and yugabyte-N (one per SP) - Update network explanations to reflect one Yugabyte per SP network - Addresses BigLep's comment about network diagram readability and accuracy
…cker compatibility - Change from YYYY-MM-DDTHH:MM to YYYYMMDDTHHMM (no dashes/colons) - Colons and dashes cause issues with Docker network names - Update code in src/run_id/mod.rs to use %Y%m%dT%H%M format - Update test regex to match new format - Update all documentation examples throughout README_ADVANCED.md - Still ISO8601-based for natural sorting, but Docker-compatible
- Convert Required Repositories table to bulleted list with repo links (avoids outdated version info) - Add Version Strategy section explaining GitTag/GitCommit/GitBranch approaches - Link to src/config.rs where default versions are defined - Link 'Updating defaults' to How Defaults Work section - Addresses BigLep's questions about version strategy and where defaults are defined
- Reduce from verbose 3-step process to simple 2-step (copy file + run init) - Remove unnecessary export and documentation steps - Addresses BigLep's comment about verbose configuration sharing
- Move Reproducible builds from under Sharing Configuration to separate section - Makes it clearer that it's a distinct topic - Addresses BigLep's comment about moving it to its own heading
- Move all flag information into Commands Reference section - Remove duplicate Command Flags section - Add inline 'why' explanations for --volumes-dir, --run-dir, and --notest - Preserve parallelization guidance (why to use/not use --parallel) - Add parallelization table to Step execution section showing which epochs are parallelized - Link from start command to Detailed Start Sequence for parallelization details - Addresses BigLep's comment about eliminating duplication
- Remove ASCII diagram - Reorganize into Before Steps/Steps/Post Start Steps structure - Remove hardcoded numbers (1-6) and letters (a-k) from step descriptions - Add links to Step Implementation Pattern and Configuration System - Add sections for running, stop, and (re)start states - Addresses BigLep's comment about improving lifecycle documentation structure
- Enhance SetupContext struct comments with detailed explanations - Add inline comments throughout example flow showing context sharing - Add method-level comments to Step trait explaining each phase - Improve code readability by explaining what each part does and why - Addresses BigLep's comment about adding explanatory content to code examples
- Rename 'Advanced Topics' to 'Additional User Actions' for clarity - Add explanation of genesis template location (~/.foc-devnet/docker/volumes/run-specific/<run-id>/genesis/) - Note that genesis templates are generated during genesis prerequisites phase - Remove 'Last Updated' section (redundant - version control tracks changes) - Addresses BigLep's comments about improving section naming and removing low-value content
- Enhance post_execute comment to include 'confirm deployment' - Remove duplicate 'Phases' section (already explained in inline code comments) - Remove redundant 'Testing scenarios' section (just comments) - Remove 'Reference Links' section (redundant with other documentation)
- Change ../src/config.rs to src/config.rs (root-level paths) - Change ../src/commands/start/ to src/commands/start/ - Change ../src/commands/start/usdfc_deploy/prerequisites.rs to src/commands/start/usdfc_deploy/prerequisites.rs - Ensure all local file links use relative paths from root directory - All relative links now correctly reference files without parent directory traversal
- Fix grammar error: 'If you are have troubles' -> 'If you have troubles' - Fix broken GitHub Issues link in System Requirements table - Update outdated 'Advanced topics' reference to 'Additional user actions' - Fix timing inconsistency: align '--parallel' timing with detailed sequence (~5 min to ~3 min) - Fix Yugabyte note: change 'shared by all SPs' to 'one per SP' and update to yugabyte-1 - Fix heading levels: change running/stop/(re)start from ### to #### under Lifecycle Overview - Fix typos: 'Regenisis' -> 'Regenesis', improve capitalization - Fix formatting: remove extra blank line after --force option - Keep latest symlink references (feature documented in code)
There was a problem hiding this comment.
Pull request overview
This PR incorporates documentation improvements from PR #16, focusing on standardizing the Run ID format to condensed ISO8601 (YYYYMMDDTHHMM) for Docker compatibility and enhancing overall documentation clarity.
Changes:
- Standardized Run ID format from
YYmmmDD-HHMM_RandomNametoYYYYMMDDTHHMM_RandomName(condensed ISO8601) - Renamed ADVANCED_README.md to README_ADVANCED.md for better lexicographical sorting
- Enhanced documentation with detailed explanations of configuration defaults, step context, network architecture, and lifecycle phases
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/run_id/mod.rs | Updated Run ID generation to use condensed ISO8601 format (YYYYMMDDTHHMM) with enhanced documentation |
| README.md | Fixed grammar, updated file references to README_ADVANCED.md, added architecture requirements callout |
| ADVANCED_README.md | Comprehensive restructuring with clarified sections, inline code comments, improved network diagrams, and standardized Run ID references |
BigLep
commented
Jan 16, 2026
redpanda-f
reviewed
Jan 17, 2026
redpanda-f
reviewed
Jan 17, 2026
redpanda-f
reviewed
Jan 17, 2026
redpanda-f
reviewed
Jan 17, 2026
redpanda-f
reviewed
Jan 17, 2026
redpanda-f
approved these changes
Jan 17, 2026
github-project-automation
bot
moved this from 🔎 Awaiting review
to ✔️ Approved by reviewer
in FOC
redpanda-f
requested changes
Jan 17, 2026
Collaborator
redpanda-f
left a comment
redpanda-f
left a comment
There was a problem hiding this comment.
Request a few changes pertaining to removal of some cli flags etc. Those would need changes on code itself.
github-project-automation
bot
moved this from ✔️ Approved by reviewer
to ⌨️ In Progress
in FOC
Contributor
Author
|
@redpanda-f : I can look at this tomorrow, but if there are just a couple of lines to remove, then do you want to take care of them? We also need a note about arm support (which we support now right?) |
- Change from 'x86 Architecture' requirement to 'Architecture' supporting both x86 and ARM64 - Update text to reflect that system automatically selects appropriate binaries - Reflects changes from PR #32 which added ARM64/Apple Silicon support
- Rename 'Repository Management' to 'Dependency Repository Management' - Rename 'Required Repositories' to 'Required Dependent Repositories' - Rename 'Version Strategy' to 'Dependent Version Strategy' - Simplify version strategy explanation - Rename 'Using Local Repositories' to 'Using Local Dependency Repositories' - Add note about using same foc-devnet commit when sharing config - Clarify reproducible builds use 'tags or commits' not just 'commits'
Remove --output-dir, --volumes-dir, and --run-dir flags as they are either unused or should not be user-configurable. The system now always uses fixed default paths: - ~/.foc-devnet/bin/ for build outputs - ~/.foc-devnet/docker-volumes/<run-id>/ for Docker volumes - ~/.foc-devnet/run/<run-id>/ for run-specific data This simplifies the CLI interface and ensures consistent behavior.
Contributor
Author
Code ChangesThis PR now includes code changes to remove unused CLI flags:
The system now always uses fixed default paths for consistency. This simplifies the CLI interface and ensures predictable behavior. |
- Format match arms on single lines where appropriate - Format function call on single line
Contributor
Author
|
@redpanda-f : I believe all comments have been addressed at this point. If there is anything else, are you good to merge this? |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
redpanda-f
approved these changes
Jan 22, 2026
Collaborator
redpanda-f
left a comment
redpanda-f
left a comment
There was a problem hiding this comment.
LGTM, thank you @BigLep
github-project-automation
bot
moved this from 🔎 Awaiting review
to ✔️ Approved by reviewer
in FOC
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR incorporates documentation comments from @BigLep on PR #16.
Changes
Related
Addresses issue #9 (documentation improvements)