Skip to content

git fetch --unshallow#67

Merged
simonw merged 1 commit intomainfrom
unshallow
Oct 18, 2025
Merged

git fetch --unshallow#67
simonw merged 1 commit intomainfrom
unshallow

Conversation

@simonw
Copy link
Owner

@simonw simonw commented Oct 18, 2025

So the Cloudflare preview builds get all the date information from the commits.

@claude
Copy link
Contributor

claude bot commented Oct 18, 2025

Claude finished @simonw's task —— View job


Code Review: git fetch --unshallow

Todo List:

  • Read and analyze the modified build.sh file
  • Check for any related CI/CD configuration files
  • Evaluate code quality and best practices
  • Assess potential bugs or issues
  • Review performance considerations
  • Check for security concerns
  • Evaluate test coverage needs
  • Provide comprehensive feedback

Review Feedback

Overall Assessment: APPROVED

This is a clean, targeted fix that addresses a specific deployment issue. The change is minimal, well-motivated, and necessary for the Cloudflare Pages build process.

📋 Code Quality & Best Practices

  • Good: The change is minimal and focused (only 3 lines added: build.sh:4-6)
  • Good: Includes helpful comment explaining the purpose: # Mae sure we have the full git history for finding commit dates:
  • Minor typo: Line 4 has "Mae" instead of "Make" - consider fixing
  • Good: Uses set -e at the top for proper error handling

🔍 Analysis of the Change

The modification adds git fetch --unshallow to convert shallow clones to full clones. This is essential because:

  1. Cloudflare Pages issue: By default, CI environments often use shallow clones (--depth=1) for performance
  2. Build dependency: The gather_links.py script (lines 18-19) uses git log --format=%H|%aI|%B%x00 to extract commit dates and metadata
  3. GitHub Pages difference: The GitHub Pages workflow (.github/workflows/pages.yml:28) already uses fetch-depth: 0 to get full history
  4. Cloudflare Pages gap: This PR ensures Cloudflare builds also have complete git history

🚀 Performance Considerations

  • Trade-off: git fetch --unshallow will increase build time and bandwidth usage
  • Justification: Necessary for the build process to extract commit metadata for the colophon
  • Alternative: Could explore caching git history between builds if performance becomes an issue

🔒 Security Assessment

  • Low risk: Standard git command with no external inputs
  • Safe context: Runs in controlled build environment
  • No concerns: No user input, secrets, or external dependencies involved

🧪 Test Coverage

  • Coverage: No direct testing needed - this is infrastructure/build configuration
  • Verification: Success can be verified by ensuring Cloudflare builds complete successfully and the colophon page displays correct commit dates
  • Monitoring: Watch for build failures or timeouts after deployment

🎯 Recommendations

  1. Fix typo: Change "Mae sure" to "Make sure" on line 4
  2. Consider documentation: Add a brief comment about why this is needed specifically for Cloudflare vs GitHub Pages
  3. Monitor: Keep an eye on Cloudflare build times after this change

Summary

This is a necessary and well-implemented fix for ensuring Cloudflare Pages builds have access to full git history. The change is minimal, safe, and directly addresses the stated problem.


@cloudflare-workers-and-pages
Copy link

Deploying tools with  Cloudflare Pages  Cloudflare Pages

Latest commit: df1ed90
Status: ✅  Deploy successful!
Preview URL: https://c24b452f.tools-b1q.pages.dev
Branch Preview URL: https://unshallow.tools-b1q.pages.dev

View logs

@simonw
Copy link
Owner Author

simonw commented Oct 18, 2025

This worked, here's the Cloudflare preview: https://c24b452f.tools-b1q.pages.dev/

CleanShot 2025-10-17 at 21 58 38@2x

@simonw simonw merged commit 8a4544f into main Oct 18, 2025
4 checks passed
@simonw simonw deleted the unshallow branch October 18, 2025 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant