Skip to content

add fuctions to load env file and replace env specific values - #60

Closed
dallinjsevy wants to merge 1 commit into
mainfrom
feature/add-replaceEnvSpecifiValues
Closed

add fuctions to load env file and replace env specific values#60
dallinjsevy wants to merge 1 commit into
mainfrom
feature/add-replaceEnvSpecifiValues

Conversation

@dallinjsevy

Copy link
Copy Markdown

Add replaceEnvScpecific values for fr-config manager placeholder support

@dallinjsevy
dallinjsevy requested a review from phalestrivir July 9, 2026 17:38
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized we should probably have tests for all these functions, especially after looking more closely at the replaceEnvSpecificValues one. We want to have tests to verify the replacing strategies, as well as reading in a JSON file, work as expected.

Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts
Comment thread src/utils/ExportImportUtils.ts
@dallinjsevy
dallinjsevy force-pushed the feature/add-replaceEnvSpecifiValues branch from cff963f to 2a0c892 Compare July 23, 2026 20:30

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add tests for the new state methods in State.test.ts, and tests for ExportImportUtils.test.ts for those functions, so make sure to do that in addition to the other things I commented on in this PR.

Comment thread src/shared/State.ts Outdated
Comment thread src/shared/State.ts Outdated
Comment thread src/shared/State.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts Outdated
Comment thread src/utils/ExportImportUtils.ts

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but we still need tests for State.test.ts and ExportImportUtils.test.ts

Comment thread src/shared/State.ts Outdated
@dallinjsevy
dallinjsevy force-pushed the feature/add-replaceEnvSpecifiValues branch from 14e8367 to 1066705 Compare August 6, 2026 16:54

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a few more tests

Comment thread src/utils/ExportImportUtils.test.ts

@phalestrivir phalestrivir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Squash your commits, rebase with main, and then submit the PR to Rockcarver

@dallinjsevy
dallinjsevy force-pushed the feature/add-replaceEnvSpecifiValues branch from f9c903c to 156379d Compare August 13, 2026 19:33
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.

2 participants