-
Notifications
You must be signed in to change notification settings - Fork 3
chore(sanitize-config): Update sanitization logic to remove only dev sections and add tests #270
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
Conversation
WalkthroughThis change updates the configuration sanitization logic across scripts and workflows to only remove the Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant Script
participant ConfigFile
Workflow->>Script: Run sanitize-config.py on config.yaml
Script->>ConfigFile: Read config.yaml
Script->>ConfigFile: Remove node.dev section
Script->>ConfigFile: Write sanitized config.yaml
Workflow->>ConfigFile: Check for presence of node.dev (error if found)
Workflow->>ConfigFile: Optionally report presence of node.admin (no error)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for genlayer-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/scripts/test_sanitize_config.py (1)
5-5: Remove unused import.The
sysimport is not used in this test script.-import sys
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/pull_request_template.md(1 hunks).github/scripts/sanitize-config.py(3 hunks).github/scripts/test_sanitize_config.py(1 hunks).github/workflows/sync-docs-from-node.yml(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: dohernandez
PR: genlayerlabs/genlayer-docs#257
File: .github/workflows/sync-docs-from-node.yml:0-0
Timestamp: 2025-07-26T13:26:45.588Z
Learning: In the sync-docs-from-node.yml workflow, the user prefers selective git staging with `git add content/validators pages/api-references pages/validators` rather than `git add -A` to avoid staging unintended changes, as these are the expected directories to be modified during the documentation sync process.
📚 Learning: applies to pages/validators/setup-guide.mdx : setup guide updates (pages/validators/setup-guide.mdx)...
Learnt from: CR
PR: genlayerlabs/genlayer-docs#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-24T07:56:58.272Z
Learning: Applies to pages/validators/setup-guide.mdx : Setup Guide Updates (pages/validators/setup-guide.mdx): Update version references in download examples and version lists, update configuration examples with new contract addresses when applicable, enhance command documentation when new features are added to existing commands, update genesis block configuration in consensus section when network upgrades occur
Applied to files:
.github/scripts/sanitize-config.py
📚 Learning: in the sync-docs-from-node.yml workflow, the user prefers selective git staging with `git add conten...
Learnt from: dohernandez
PR: genlayerlabs/genlayer-docs#257
File: .github/workflows/sync-docs-from-node.yml:0-0
Timestamp: 2025-07-26T13:26:45.588Z
Learning: In the sync-docs-from-node.yml workflow, the user prefers selective git staging with `git add content/validators pages/api-references pages/validators` rather than `git add -A` to avoid staging unintended changes, as these are the expected directories to be modified during the documentation sync process.
Applied to files:
.github/workflows/sync-docs-from-node.yml
🪛 Ruff (0.12.2)
.github/scripts/test_sanitize_config.py
5-5: sys imported but unused
Remove unused import: sys
(F401)
🔇 Additional comments (10)
.github/pull_request_template.md (1)
1-27: LGTM! Excellent addition for standardizing contributions.This PR template follows conventional commit standards and provides clear guidance for contributors. The examples and type definitions are comprehensive and will help maintain consistent PR titles across the repository.
.github/scripts/sanitize-config.py (3)
3-6: LGTM! Documentation correctly updated.The docstring and comments accurately reflect the new behavior of removing only dev sections while preserving admin sections.
37-37: LGTM! Function documentation updated correctly.The function docstring now accurately describes that only
node.devsections are removed.
52-90: LGTM! Logic correctly updated to target only dev sections.The sanitization logic has been properly updated to:
- Only search for
dev:lines (Line 59)- Maintain the same validation logic to ensure it's under a
node:section- Preserve all other sections including
admin:The implementation is consistent with the PR objectives.
.github/workflows/sync-docs-from-node.yml (4)
309-309: LGTM! Comment updated to reflect new sanitization scope.The comment correctly describes that only
node.devsections are removed, aligning with the updated sanitization behavior.
346-346: LGTM! Verification check updated correctly.The grep check now only looks for
dev:sections, consistent with the new sanitization logic that preservesadmin:sections.
432-432: LGTM! Admin section now reported without error.The check for admin sections now reports their presence informatively rather than treating them as an error, which aligns with the updated behavior of preserving admin sections.
728-728: LGTM! Final verification updated correctly.The final verification step now only checks for
dev:sections as an error condition, properly reflecting the updated sanitization logic..github/scripts/test_sanitize_config.py (2)
8-14: LGTM! Dynamic import approach works well for testing.The dynamic import using
importlib.utilis a clean approach for importing the sanitize_config function from the script file for testing.
16-95: Excellent comprehensive test coverage!This test thoroughly validates the new sanitization behavior by:
- Creating a realistic config with both
adminanddevsections undernode:- Verifying that only
devsections are removed whileadminsections are preserved- Checking that other sections (
rpc,ops,genvm) remain intact- Including proper cleanup with temporary file handling
The test cases cover all the key requirements from the PR objectives and provide confidence that the sanitization logic works correctly.
Description
This PR updates the configuration sanitization logic in the documentation sync workflow to preserve the
adminsection while only removing thedevsection from the GenLayer node configuration files. Previously, bothadminanddevsections were being removed during the documentation sync process.The change includes:
sanitize-config.pyto only target and removedev:sectionstest_sanitize_config.pyto verify the sanitization behaviorSummary by CodeRabbit
New Features
Documentation
Chores
devsection in configuration files, leaving theadminsection unaffected.