paranext-core part of suppporting dev env support folder switching#2283
Open
katherinejensen00 wants to merge 2 commits into
Open
paranext-core part of suppporting dev env support folder switching#2283katherinejensen00 wants to merge 2 commits into
katherinejensen00 wants to merge 2 commits into
Conversation
- Capitalize brand name in getAppDir TSDoc ("Platform.Bible folder")
- Remove static PRODUCT_FOLDER_NAME import from util.test.ts; inline literal string to stay consistent with vi.resetModules() pattern
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Code Review Summary
Branch: Fix-manual-sync-in-dev
Base: origin/main
Date: 2026-05-15
Review model: Claude Sonnet 4.6
Files changed: 3
Overview
This change is a focused refactor that extracts the hardcoded string
'platform.bible'(the user app-data folder name) into a named exported constantPRODUCT_FOLDER_NAMEinsrc/node/utils/util.ts. ThegetAppDirfunction now derives its path from this constant rather than a duplicate string literal, and the corresponding test is updated to match. Thelib/papi-dts/papi.d.tstype surface is updated to reflect the new export.This is part of a larger effort to correctly identify and use the right application data path depending on whether Platform.Bible is running in a development or production environment. The change is a pure refactor —
getAppDirproduces the same path value as before and there is no behavioral change at runtime.A secondary fix was included: a stale
{{ productInfo.name }}template placeholder in the TSDoc forgetAppDirwas replaced with the actual resolved value.platform.bible.API Changes
node/utils/util(inpapi.d.ts): AddedPRODUCT_FOLDER_NAMEexport (string constant'platform.bible')node/utils/util(inpapi.d.ts): Updated TSDoc comment ongetAppDir— replaced stale template variable{{ productInfo.name }}with resolved value.platform.bibleFindings
Critical — Must address before merge
None.
Important — Should address before merge
None.
Minor — Consider
src/node/utils/util.ts:64— TSDoc summary forgetAppDirused lowercase "platform.bible folder"; the brand name is "Platform.Bible" (title-case). The constant's own TSDoc correctly used "Platform.Bible". (fixed during review: changed "platform.bible folder" to "Platform.Bible folder" in the TSDoc summary line)src/node/utils/util.test.ts:4— Static top-level import ofPRODUCT_FOLDER_NAMEwas a mild inconsistency with thevi.resetModules()+ dynamicimport()pattern used in the test file. (fixed during review: removed the static import and inlined'.platform.bible'in the test assertion to stay fully consistent with the existing pattern)[Both minor findings were fixed during the review interview. No open findings remain.]
Template Propagation
Shared Regions Modified
None.
Extension Config Changes
None.
Positive Observations
PRODUCT_FOLDER_NAMEas a named constant eliminates duplicated magic string literals and makes a future folder-name change a one-line update.{{ productInfo.name }}template placeholder in thegetAppDirTSDoc was correctly replaced with the actual resolved value — good documentation accuracy fix.papi.d.tswas updated consistently with the new export, keeping the public type surface in sync.getAppDir— the produced path was already.platform.bible, so this is a zero-risk refactor.path.join(os.homedir(), '/.platform.bible')had a redundant leading/on the second argument; the template literal form using the constant removes this minor inconsistency.Interview Notes
Author's stated purpose: part of a larger effort to correctly identify and use the right application data path depending on whether Platform.Bible is running in development or production. No Critical or Important findings were raised. Two minor findings (TSDoc capitalization, test import consistency) were fixed during the review interview. The author had no additional context to add.
In-Review Quality Check
Two files were modified during the review interview (
src/node/utils/util.ts,src/node/utils/util.test.ts).lib/platform-bible-react/dist/index.d.tsand@types/react, unrelated to these changes. Zero new errors in the changed files.build:typesprerequisite step — same pre-existing root cause. No errors in either changed file when linting scripts directly.src/node/utils/util.test.ts): Both tests pass:getAppDir > returns .platform.bible in home directory when packagedgetAppDir > returns resourcesPath/dev-appdata when not packagedNo auto-fixes were applied. The pre-existing typecheck/lint failures are not related to this branch.
Suggested Review Focus
papi.d.tswas regenerated viabuild:typesrather than hand-edited (the diff is consistent with regeneration, but worth verifying the generation pipeline was run).PRODUCT_FOLDER_NAMEfrom, given the larger effort around app data path management — or whether it should eventually live in a more prominent shared location.This change is