docs: improve DROP DATABASE diagram clarity#30
Open
ebembi-crdb wants to merge 16 commits into
Open
Conversation
This commit introduces an automated pipeline for SQL diagram generation that simplifies the workflow for technical writers. Changes: - Add CI workflow (.github/workflows/docgen-diagrams-ci.yml) that: - Triggers on changes to diagrams.go, sql.y, or the docgen-diagram label - Generates BNF files from sql.y - Builds SVG diagrams via Railroad Diagram Generator - Runs comprehensive validation tests - Packages and uploads artifacts - Triggers bot to sync diagrams to generated-diagrams repo - Comments on PRs with validation status - Add validation test suite (pkg/cmd/docgen/tests/): - TestMissingBNFFiles: Detects missing BNF files - TestMissingHTMLDiagrams: Detects missing HTML output - TestStmtNamingConflicts: Detects _stmt naming conflicts - TestReplaceUnlinkMismatch: Warns about potential broken links - TestBrokenGrammarLinks: Finds invalid grammar references - TestUnusedDiagrams: Reports diagrams not used in docs - TestSKIPDOCSuppressions: Reports SKIP DOC annotations - Add bot sync script (build/scripts/sync-diagrams-to-generated-repo.sh): - Clones generated-diagrams repository - Updates BNF and SVG files - Creates PR with validation summary (no auto-merge) - Assigns PR to docs-infra-prs team - Add writer documentation (docs/generated/sql/bnf/README.md): - Instructions for adding/modifying diagrams - Explains what writers don't need to do (no Bazel commands) - Troubleshooting guide - Add technical documentation (docs/tech-notes/sql-diagram-automation.md): - Architecture overview with diagram - CI workflow details - Bot service specification - Secret configuration - Troubleshooting and rollback procedures New writer workflow: 1. Edit pkg/cmd/docgen/diagrams.go only 2. Open PR with label docgen-diagram 3. Add remote_include manually only for new diagrams Release Notes: None Epic: None
…t repo This commit improves the SQL diagram automation CI workflow: 1. Added changed_files output to track which diagrams changed 2. Updated manifest.json to include changed files as a JSON array 3. Made target repository configurable via DIAGRAM_TARGET_REPO variable (defaults to cockroachdb/generated-diagrams) 4. Enhanced PR comment to show list of changed diagrams 5. Updated TestUnusedDiagrams stub to clarify it's a placeholder for future functionality Epic: None Release note: None
This commit removes the pull_request_target event trigger from the SQL diagram generation workflow to address a security vulnerability. The issue was that pull_request_target grants access to secrets and runs with write permissions. When combined with checking out untrusted PR code and executing scripts from it (./dev, ./build/github/*.sh), this created a vector for secret exfiltration and runner compromise. Changes: - Remove pull_request_target trigger (used for docgen-diagram label) - Remove label check from should_run logic - Add comment explaining the security rationale - workflow_dispatch remains available for manual triggering by maintainers The pull_request trigger (for path changes) remains safe since it does not have access to secrets. Epic: None Release note: None
docgen: Implement complete SQL diagram automation pipeline
This test change adds 'opt_collate' to the inline list for the add_column diagram to demonstrate the SQL diagram automation workflow. Epic: None Release note: None
This commit is FORK-SPECIFIC and should NOT be upstreamed. Changes for running on forks without CockroachDB infrastructure: - Use ubuntu-latest instead of self-hosted runners - Add Bazel installation - Skip EngFlow keys (internal to CockroachDB) Epic: None Release note: None
Testing the automated SQL diagram generation workflow with GitHub-hosted runners. Epic: None Release note: None
- Add Java 11 setup (required for Railroad JAR) - Download and configure Railroad Diagram Generator JAR - Set COCKROACH_REQUIRE_RAILROAD environment variable - Add fallback build command with explicit JAR location This should enable HTML/SVG diagram generation on GitHub-hosted runners. Epic: None Release note: None
The unzip was conflicting with existing LICENSE file in the repo. Now extracting in a temporary directory to avoid conflicts. Epic: None Release note: None
Adding diagnostic output to understand why HTML files aren't being generated. Epic: None Release note: None
Testing the workflow with debug output to see why HTML files aren't generated. Epic: None Release note: None
The dev wrapper requires initialization which doesn't work well on GitHub-hosted runners. Using bazel directly for: - BNF generation - SVG diagram building - Validation tests This should fix the HTML generation issue. Epic: None Release note: None
Replace generic 'name' with 'database_name' in the DROP DATABASE diagram to make it clearer for users. Release note: None Epic: None
SQL Diagram Generation Report✅ Validation Passed 🔄 Diagram changes detected SKIP DOC Warnings (click to expand)The following grammar rules are suppressed from documentation: This comment was generated by the SQL Diagram CI workflow. |
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
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.
Summary
Improve the DROP DATABASE diagram by replacing the generic grammar term
namewith the clearer labeldatabase_name.Before: Diagram shows
name(confusing - what name?)After: Diagram shows
database_name(clear and specific)This is a typical writer workflow - making diagrams more user-friendly.
Release note: None
Epic: None