Skip to content

Add blog link to Open Sauce page#39

Merged
simonw merged 3 commits intomainfrom
codex/add-link-to-how-i-built-this
Jul 18, 2025
Merged

Add blog link to Open Sauce page#39
simonw merged 3 commits intomainfrom
codex/add-link-to-how-i-built-this

Conversation

@simonw
Copy link
Owner

@simonw simonw commented Jul 18, 2025

Summary

  • link to blog post on how the page was built
  • update docs commit reference

Testing

  • pytest -q (fails: ModuleNotFoundError: No module named 'playwright')

https://chatgpt.com/codex/tasks/task_e_687951b3ce6c8326be8d0f1d5518a918

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jul 18, 2025

Deploying tools with  Cloudflare Pages  Cloudflare Pages

Latest commit: a0de377
Status:⚡️  Build in progress...

View logs

@claude
Copy link
Contributor

claude bot commented Jul 18, 2025

Claude finished @simonw's task —— View job


PR Review Complete ✅

Reviewing PR #39: "Add blog link to Open Sauce page"

Todo List

  • Read and understand the changed files
  • Analyze changes for code quality and best practices
  • Check for potential bugs or issues
  • Review performance considerations
  • Assess security concerns
  • Evaluate test coverage impact
  • Provide comprehensive review feedback

Review Analysis

Changes Summary

The PR makes two simple but meaningful changes:

  1. open-sauce-2025.html:243 - Adds a "How I built this" link pointing to https://simonwillison.net/2025/Jul/17/vibe-scraping/
  2. open-sauce-2025.docs.md - Simplifies the description and removes the commit reference

Code Quality & Best Practices ✅

  • The HTML addition is well-structured and follows existing patterns
  • Link is properly placed in the header section with consistent styling
  • Documentation update improves clarity and conciseness
  • Inline styling is consistent with the existing codebase approach

Potential Issues ⚠️

Security Recommendation: The external link should include security attributes:

<a href="https://simonwillison.net/2025/Jul/17/vibe-scraping/" 
   style="color: white" 
   target="_blank" 
   rel="noopener noreferrer">How I built this</a>

Accessibility: Consider adding aria-label for screen readers:

<a href="..." aria-label="Read blog post about how this page was built">How I built this</a>

Performance Considerations ✅

  • Minimal impact: single text link adds negligible overhead
  • No additional resources or scripts required
  • DNS prefetch could be added but unnecessary for single link

Security Assessment ⚠️

  • Missing security attributes: External links should include target="_blank" rel="noopener noreferrer" to prevent:
    • Window object access via window.opener
    • Potential tabnabbing attacks
  • Link destination appears legitimate (simonwillison.net domain)

Test Coverage 📝

  • PR mentions pytest -q fails due to missing playwright module
  • The simple link addition shouldn't break existing functionality
  • Consider adding a basic HTML validation test if none exists
  • Link could be tested for proper href attribute and styling

Recommendations

  1. High Priority: Add security attributes to the external link
  2. Medium Priority: Consider accessibility improvements
  3. Low Priority: Ensure tests can run with proper dependencies

Overall Assessment

The changes are straightforward and valuable - providing transparency about the page's creation process. The main concern is the missing security attributes on the external link, which should be addressed for best practices.

Verdict: ✅ Approve with suggested security improvements


@simonw simonw merged commit fab06a5 into main Jul 18, 2025
3 of 4 checks passed
@simonw
Copy link
Owner Author

simonw commented Jul 18, 2025

@simonw simonw deleted the codex/add-link-to-how-i-built-this branch July 18, 2025 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant