init-action: save updated config #3109
Conversation
This commit updates the init action to save the config again at the end of run(), so that config updates in run() are correctly propagated to the analyze action.
There was a problem hiding this comment.
Pull Request Overview
This PR addresses a configuration propagation issue in the init action by saving the updated config at the end of the run() function to ensure config changes are properly passed to the analyze action.
- Exports the
saveConfigfunction in config-utils.ts to make it publicly available - Adds a call to save the config after successful initialization and before sending the status report
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/config-utils.ts | Exports the saveConfig function to make it accessible from other modules |
| src/init-action.ts | Adds config saving after initialization to ensure updated config is persisted for the analyze action |
| lib/init-action.js | Generated JavaScript code (not reviewed per guidelines) |
There was a problem hiding this comment.
Thanks for noticing this and implementing a fix!
I am wondering whether we can remove the saveConfig call at the end of initConfig. There'll probably be some tests that would need changing, but I can't immediately think of a good reason why the config has to be saved that early.
If we are adding this later call (which seems like it's necessary given the potential changes to the config after initConfig), I'd ideally have this be the only place where we save the config.
| ); | ||
|
|
||
| // The saved config file should now exist | ||
| t.true(fs.existsSync(configUtils.getPathToParsedConfigFile(tempDir))); |
There was a problem hiding this comment.
It might be possible to keep this test if you now manually call saveConfig before this check?
There was a problem hiding this comment.
The stated purpose of the test is to verify that initConfig() saves the config file. After the suggested change, initConfig() no longer saves the config file.
Can you clarify how you would like to see this test saved?
There was a problem hiding this comment.
I think this test is more of a round-trip test: whether saving a config and then loading it results in the same object. It just so happened that initConfig was saving the config before, but not what was primarily being tested.
cklin
force-pushed
the
cklin/init-save-updated-config
branch
from
05b2852 to
5c30ae4
Compare
This PR updates the init action to save the config again at the end of
run(), so that config updates inrun()are correctly propagated to the analyze action.Risk assessment
For internal use only. Please select the risk level of this change:
Merge / deployment checklist