chore: add upgrade step while processing resume #66
Conversation
WalkthroughThe PR modernizes the JSON Resume theme by introducing schema migration logic to automatically upgrade outdated resume formats, comprehensively refactoring the HTML template for consistency and clarity, updating test fixtures, and adjusting TypeScript configuration. No behavioral breaking changes; backward compatibility is maintained through the upgrade mechanism. Changes
Sequence DiagramsequenceDiagram
participant User
participant render as render()
participant upgrade as upgradeOutdatedResume()
participant template as Handlebars Template
User ->> render: render(outdatedResume)
render ->> upgrade: upgradeOutdatedResume(resume)
activate upgrade
upgrade ->> upgrade: Migrate field names<br/>(company→name, picture→image, etc.)
upgrade -->> render: returns boolean (upgraded?)
deactivate upgrade
alt Upgrade occurred
render ->> render: log warning
end
render ->> template: pass upgraded resume
template -->> render: HTML output
render -->> User: rendered resume
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The PR spans multiple file types with varying complexity: the schema migration logic is new and requires validation; the template restructuring is extensive but follows consistent patterns throughout sections; config and fixture changes are straightforward. The heterogeneous nature of edits (logic, template, config) demands separate reasoning per cohort, though individual changes within cohorts are largely repetitive or conventional. Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tsconfig.json (1)
14-14: Consider re-enablingnoImplicitAnyfor better type safety.Setting
noImplicitAny: falseallows implicitanytypes, which can mask type errors and reduce the benefits of TypeScript's type checking. Unless there's a specific reason for disabling this (e.g., gradual migration), consider keeping it enabled or adding a comment explaining the rationale.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(2 hunks)index.js(3 hunks)resume.handlebars(1 hunks)test/fixture.resume.json(1 hunks)tsconfig.json(1 hunks)
🔇 Additional comments (12)
tsconfig.json (1)
6-8: LGTM!Formatting change to multiline array is consistent and improves readability.
README.md (1)
46-47: LGTM!These are trivial Markdown formatting changes (list marker style and trailing whitespace) with no semantic impact.
Also applies to: 61-61
index.js (6)
78-92: Well-documented upgrade function.The JSDoc clearly explains the purpose, parameters, return value, and mutation behavior. The references to specific schema releases are helpful for understanding the migration history.
95-128: LGTM: Basics-level migrations look correct.The migration logic properly:
- Preserves existing new-format fields with
!resume.basics.fieldchecks- Handles the
firstName/lastName→nameconcatenation gracefully- Maps all the common legacy field names to their v1.0.0 equivalents
The logic is defensive and won't overwrite manually migrated fields.
130-160: LGTM: Array entry migrations are well-structured.The migrations for work, volunteer, and publications entries:
- Properly iterate arrays with guards (
Array.isArray())- Use safe property access with optional chaining (
work?.company)- Preserve existing new-format fields
- Track upgrades correctly
194-196: LGTM: Clean integration with helpful warning.The upgrade is performed early (before any template processing), and the warning message is clear and actionable, directing users to the schema documentation.
198-224: LGTM: Improved profile processing with proper guards.The refactored profile processing:
- Uses defensive checks with
Array.isArray(resume.basics?.profiles)- Properly scopes the
profilesdestructuring- Maintains the existing xTwitter handle extraction logic
This prevents potential runtime errors when
basicsorprofilesis undefined or not an array.
167-179: The review comment is based on incorrect assumptions about the schema history and should be disregarded.v0.0.12 schema for languages already uses "language" and "fluency" fields, not "name" and "level". The migration code correctly maps
name→languageandlevel→fluencyto upgrade from even older pre-v0.0.12 versions to the v0.0.12+ format. The mappings are not backwards—they represent a forward upgrade to the current schema standard.Likely an incorrect or invalid review comment.
resume.handlebars (4)
8-45: LGTM: Well-structured head metadata.The head section properly:
- Provides fallback title when
basics.nameis missing- Populates Open Graph and Twitter Card metadata
- Guards all optional fields with conditionals
- Includes proper charset, viewport, and accessibility attributes
49-116: LGTM: Basics section aligned with v1.0.0 schema.The basics section correctly:
- Uses
nameandlabelfields (lines 51-54)- References
urlinstead of legacywebsite(line 62)- Guards the profiles section with
profiles.length(line 89)- Provides fallback rendering (e.g., URL shown when username missing, lines 101-105)
118-164: LGTM: Work section uses correct v1.0.0 schema fields.The work section correctly uses:
namefield for the company/organization name (lines 123-127)urlinstead of legacywebsite(lines 142-146)- Proper guards for all optional fields
- Markdown rendering for
summaryandhighlightsThis aligns with the upgrade logic in index.js that converts legacy
company→name.
166-359: LGTM: Remaining sections well-structured and schema-compliant.All remaining sections (volunteer, education, skills, languages, interests, references):
- Use proper v1.0.0 schema field names (e.g.,
language/fluencyon lines 306-312 instead of legacy alternatives)- Include appropriate guards for optional content
- Follow consistent rendering patterns
- Properly use the markdown helper for rich text fields
The template comprehensively covers all JSON Resume sections with clean, maintainable markup.
| "work": [ | ||
| { | ||
| "name": "JSON Resume", | ||
| "company": "JSON Resume", |
There was a problem hiding this comment.
Critical: Test fixture uses outdated schema format.
This change introduces an inconsistency. The fixture is being changed to use the OLD schema field "company" instead of the NEW v1.0.0 schema field "name". This is backwards because:
- Line 2 explicitly references the v1.0.0 schema:
"$schema": "https://raw.githubusercontent.com/jsonresume/resume-schema/v1.0.0/schema.json" - The upgrade logic in index.js (lines 132-135) converts
company→name(treatingcompanyas legacy) - The template resume.handlebars (lines 123-127) expects the
namefield
Test fixtures should demonstrate the current/correct schema format, not legacy formats that require upgrading.
Apply this diff to use the correct v1.0.0 schema field:
- "company": "JSON Resume",
+ "name": "JSON Resume",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "company": "JSON Resume", | |
| "name": "JSON Resume", |
🤖 Prompt for AI Agents
In test/fixture.resume.json around line 30, the fixture uses the legacy field
"company" instead of the v1.0.0 schema field "name"; update the JSON fixture to
replace the "company" key with "name" (preserve the same value "JSON Resume") so
the fixture matches the declared schema and the template/upgrade logic
expectations.
1 participant
In the
resume.handlebars, we had a couple ifif/else ifblocks, where we were keeping support for some outdated property names.For example,
work.nameused to bework.company. Some resumes in the wild still use the old name, so we've kept support.However, it's a little messy and error-prone to maintain this in the view, this should probably be in a preprocessing step that runs before we start rendering the resume in the first place.
This introduces that preprocessing step,
#upgradeOutdatedResume. This will rewrite outdated properties to whatever it should look like in the v1.0.0 specifications. Ofc, properties that are not known to use can not be acted on.This should handle is gracefully, but does also log a warning guiding developers to read the schema.
Here is what I see when I build a resume that has
work.companyinstead ofwork.name:We originally only maintained 4 fallbacks, which happen to be the ones this resume was using by name before. However, I'd actually gone back in the release history and support for upgrading old property names.
I didn't add everything, but I'm prepared to add more if we encounter any other problems regarding compatibility.
Chores
Summary by CodeRabbit
New Features
Chores