Skip to content

Specific methods for patch app configuration file scenarios #5641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shauns
Copy link
Contributor

@shauns shauns commented Apr 11, 2025

WHY are these changes introduced?

The current implementation of patchAppConfigurationFile is too broad, and we'll need something more specific if we want to switch out for a format preserving patcher.

WHAT is this pull request doing?

This PR introduces three new, more intuitive functions for modifying app configuration:

  1. setAppConfigValue - Sets a single value in the app configuration using a dotted path notation
  2. unsetAppConfigValue - Removes a value from the app configuration
  3. setManyAppConfigValues - Sets multiple values at once in the app configuration

The original patchAppConfigurationFile function is now marked as internal, and all existing usages have been updated to use the new functions. This provides a more declarative API for modifying configuration values.

How to test your changes?

  1. Run the test suite to verify all tests pass
  2. Try using the CLI to modify app configuration values (e.g., dev command that updates URLs)
  3. Verify that the TOML file is correctly updated with the expected values

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

Copy link
Contributor Author

shauns commented Apr 11, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

github-actions bot commented Apr 11, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
76.73% (+0.06% 🔼)
9501/12383
🟡 Branches
71.77% (+0.01% 🔼)
4657/6489
🟡 Functions
76.51% (+0.06% 🔼)
2462/3218
🟡 Lines
77.27% (+0.06% 🔼)
8986/11630
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / context.ts
78.26% (-0.37% 🔻)
65.67% 79.17%
80.81% (-0.38% 🔻)
🟢
... / patch-app-configuration-file.ts
100%
62.5% (-37.5% 🔻)
100% 100%

Test suite run success

2211 tests passing in 961 suites.

Report generated by 🧪jest coverage report action from 953e0ef

@shauns shauns force-pushed the shauns/04-08-specific_methods_for_patch_app_configuration_file_scenarios branch from fe1f0b4 to 767880c Compare April 22, 2025 13:42
@shauns shauns marked this pull request as ready for review April 22, 2025 13:46
@shauns shauns requested a review from a team as a code owner April 22, 2025 13:46
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

@shauns shauns force-pushed the shauns/04-08-specific_methods_for_patch_app_configuration_file_scenarios branch from 767880c to 953e0ef Compare April 22, 2025 13:51
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.

1 participant