Skip to content

Vendor example app includes as local files#23405

Merged
rmloveland merged 2 commits into
mainfrom
2026-05-28-DOC-17061-remote_include-example-apps
May 29, 2026
Merged

Vendor example app includes as local files#23405
rmloveland merged 2 commits into
mainfrom
2026-05-28-DOC-17061-remote_include-example-apps

Conversation

@rmloveland
Copy link
Copy Markdown
Contributor

@rmloveland rmloveland commented May 28, 2026

Fixes DOC-17061

Summary of changes:

  • Vendored example app source snippets into local include files.

  • Replaced example app remote includes with local includes.

  • Added scripts to vendor and validate example app includes.

@rmloveland rmloveland requested a review from a team as a code owner May 28, 2026 19:52
@rmloveland rmloveland requested a review from ebembi-crdb May 28, 2026 19:52
@netlify
Copy link
Copy Markdown

netlify Bot commented May 28, 2026 •
edited
Loading

✅ Deploy Preview for cockroachdb-api-docs ready!

Name Link
🔨 Latest commit c47489d
🔍 Latest deploy log https://app.netlify.com/projects/cockroachdb-api-docs/deploys/6a19e453c4363b0008679c2a
😎 Deploy Preview https://deploy-preview-23405--cockroachdb-api-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 28, 2026 •
edited
Loading

Files changed:

@netlify
Copy link
Copy Markdown

netlify Bot commented May 28, 2026 •
edited
Loading

✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit c47489d
🔍 Latest deploy log https://app.netlify.com/projects/cockroachdb-interactivetutorials-docs/deploys/6a19e453336f330008e9e371

@rmloveland
Copy link
Copy Markdown
Contributor Author

rmloveland commented May 28, 2026

Related to #23297.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 28, 2026 •
edited
Loading

✅ Netlify Preview

Name Link
🔨 Latest commit c47489d
🔍 Latest deploy log https://app.netlify.com/projects/cockroachdb-docs/deploys/6a19e4536abc590008dd6972
😎 Deploy Preview https://deploy-preview-23405--cockroachdb-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ebembi-crdb ebembi-crdb mentioned this pull request May 29, 2026
Open
3 tasks
@ebembi-crdb
Copy link
Copy Markdown
Contributor

ebembi-crdb commented May 29, 2026

Review: PR #23405 — Vendor example app includes as local files

Overview

This PR vendors 60 source files from 8+ repos (Rust, Go, Java, Node.js, Python, C#, Spring) and updates 156 markdown pages across 12 version directories (v23.1 through v26.3). It adds 3 Python scripts for vendoring, verification, and validation.

Key findings

Strengths:

  • Well-designed vendoring script (vendor_example_app_includes.py) with:
    • Proper retry logic (3 attempts with exponential backoff)
    • Concurrent fetching (16 workers with URL deduplication)
    • HTML comment awareness (skips remote_include tags inside comments)
    • Collision detection for local include paths
    • Self-verification after rewriting
  • Correct snippet extraction between start/end markers — verified Rust (BEGIN execute_txn/END execute_txn), Python (# START front/# END front), and Go snippets against upstream
  • Separate verify script for CI validation, checking remaining remote includes, missing files, stale files, and empty files
  • Comprehensive coverage: 603 remote_include → local include substitutions across all versions
  • Vendored file paths preserve owner/repo/ref/path hierarchy for traceability
  • Complete 1:1 substitutions preserving surrounding code blocks and copy-clipboard includes

Minor suggestions (non-blocking):

  1. CI workflow: No GitHub Actions workflow is set up to run validate_example_app_includes.py on PRs. This would prevent regressions. Is this planned for a follow-up?

  2. validate vs verify scripts: Both do the same thing — validate_example_app_includes.py is a 20-line wrapper around verify_example_app_includes.py using runpy.run_path(). Consider consolidating or clarifying the intended distinction.

  3. marker_slug empty-string fallback (cosmetic): If marker text strips to empty, sha256("") always yields the same hash. In practice parse_markers() rejects empty markers upstream, but the fallback is misleading. Consider raising an error instead.

  4. No overall script timeout: If a worker hangs despite the 30-second per-request timeout, as_completed() will block indefinitely. Consider adding a timeout parameter.

  5. Upstream freshness: Once vendored, files are static snapshots. Consider documenting the process for re-vendoring when upstream repos change.

Spot-check results

  • Rust snippets (execute_txn, transfer_funds): match upstream markers ✓
  • Go GORM main.go: matches upstream ✓
  • MovR Flask models__front.py: correct extraction between markers ✓
  • All 603 substitutions preserve context correctly ✓

LGTM with minor suggestions above. @rmloveland

rmloveland added 2 commits May 29, 2026 15:08
@rmloveland
Vendor example app includes as local files
ef2b5dc 
Fixes DOC-17061

Summary of changes:

- Vendored example app source snippets into local include files.

- Replaced example app remote includes with local includes.

- Added scripts to vendor and validate example app includes.
@rmloveland
Remove initial sync/validation scripts in favor of future EDUENG work
c47489d
@rmloveland rmloveland force-pushed the 2026-05-28-DOC-17061-remote_include-example-apps branch from 70b1ab9 to c47489d Compare May 29, 2026 19:09
@rmloveland rmloveland enabled auto-merge May 29, 2026 19:09
@rmloveland
Copy link
Copy Markdown
Contributor Author

rmloveland commented May 29, 2026

Thanks for the review. We decided to remove the initial sync/validation scripts from this PR in favor of future EDUENG work on a better-engineered solution for ongoing sync and validation. This PR now just vendors the example app includes as local files and updates the docs include sites.

@rmloveland
Copy link
Copy Markdown
Contributor Author

rmloveland commented May 29, 2026

Filed follow-up EDUENG work for the supported sync/validation path here: https://cockroachlabs.atlassian.net/browse/EDUENG-765

@rmloveland rmloveland merged commit 59f1cad into main May 29, 2026
8 checks passed
@rmloveland rmloveland deleted the 2026-05-28-DOC-17061-remote_include-example-apps branch May 29, 2026 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ebembi-crdb ebembi-crdb Awaiting requested review from ebembi-crdb

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rmloveland @ebembi-crdb