这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@SethFalco
Copy link
Member

@SethFalco SethFalco commented Oct 26, 2025

Does a few small changes.

  1. Skips the Open Graph image if we didn't get an http/https protocol URL.
  2. Adds a mock favicon for testing.

I decided not to base64 encode/inline the favicon when it's a file:// protocol URL:

  • Node.js natively only supports file:// for absolute URLs. (Though it wouldn't be hard to tack on support for relative.)
  • Then there are some issues with not knowing where the file is relative from, since we get the contents of the resume but not the original location of the resume. (A workaround for this is to treat the path as relative to where resume-cli was invoked, rather than relative to the resume.json file, though this is unintuitive.)

I figured until someone comes forward to complain about it otherwise, I'll refrain from taking any action here. I'll skip the og:image when it's not an HTTP url, otherwise we'll just inline it into the document exactly at the user provided and assume they know what they're doing.

Related

Summary by CodeRabbit

  • Bug Fixes

    • Improved Twitter/X handle extraction with stricter URL validation for more accurate profile identification.
  • Improvements

    • Separated Open Graph image configuration from favicon handling for better social media sharing control.
  • Tests

    • Updated test fixtures to support image metadata validation scenarios.

@coderabbitai
Copy link

coderabbitai bot commented Oct 26, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

The changes initialize resume.custom as an empty object, refine Twitter/X handle extraction with anchored regex matching, transition from object replacement to nested property mutation, add conditional logic to set resume.custom.ogImage when resume.basics.image is a valid HTTP(S) URL, and restructure template rendering to separately handle favicon and og:image meta tags based on image source type.

Changes

Cohort / File(s) Summary
Image and custom property initialization
src/index.js
Initializes resume.custom as empty object; refactors Twitter/X handle extraction to use anchored regex pattern; changes custom property mutation strategy from object replacement to nested property assignment; adds conditional block to set resume.custom.ogImage when resume.basics.image is a valid HTTP(S) URL; adds type annotation comment for resume.custom.
Template rendering restructuring
src/resume.handlebars
Decouples favicon and og:image handling into separate conditional blocks; favicon now set via single-line link tag; introduces conditional block for resume.custom.ogImage to supply og:image meta tag; relocates og:image:alt generation under resume.basics.name condition with modified i18n key.
Test data fixture
test/fixture.resume.json
Adds image field under basics object with value "./test/favicon.png".

Sequence Diagram

sequenceDiagram
    participant Input as Resume JSON
    participant Processing as index.js
    participant Custom as resume.custom
    participant Template as resume.handlebars
    participant Output as HTML Output

    Input->>Processing: basics.image provided
    alt basics.image is HTTP(S) URL
        Processing->>Custom: Set ogImage property
        Custom-->>Template: ogImage available
        Template->>Output: Render og:image meta tag
    else basics.image is file:// URL
        Processing->>Processing: Resolve & base64 encode
        Custom-->>Template: ogImage not set
        Template->>Output: Use basics.image for favicon only
    end
    Template->>Output: Render favicon link tag
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring extra attention:

  • Verify the anchored Twitter/X regex pattern (^) correctly restricts URL matching to expected formats
  • Confirm resume.custom.ogImage assignment only occurs when resume.basics.image matches HTTP(S) protocol validation logic
  • Validate template conditional nesting and that both favicon and og:image rendering paths work correctly without unintended side effects
  • Ensure test fixture addition (basics.image) aligns with expected file path resolution behavior

Possibly related PRs

Poem

🐰 Ears twitching with delight

Custom objects now sprouting roots,
Images sorted—HTTPS gets og glory,
Files fade to favicon boots,
Twitter patterns sharper, crisper story!
Hops away contentedly

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b3246a8 and c504e0a.

⛔ Files ignored due to path filters (1)
  • test/favicon.png is excluded by !**/*.png
📒 Files selected for processing (3)
  • src/index.js (3 hunks)
  • src/resume.handlebars (1 hunks)
  • test/fixture.resume.json (1 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@SethFalco SethFalco merged commit 4e38452 into jsonresume:main Oct 26, 2025
2 of 3 checks passed
@SethFalco SethFalco deleted the fix-og-image branch October 26, 2025 01:17
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.

If basics.image resolves to a file that exists, base64 encode and inline it

1 participant