fix: extract embedded defaults during dry runs#278
Conversation
There was a problem hiding this comment.
No—it became unused once default extraction was made unconditional. Removed it from LoadProjectConfig and the forwarding app method in 25dcdec.
25dcdec to
267fa71
Compare
There was a problem hiding this comment.
Pull request overview
Fixes dry-run failures during project configuration loading by ensuring embedded default configs are extracted into the temp directory even when the CLI is invoked with --dry-run, while keeping “ordinary” directory copies suppressed in dry runs.
Changes:
- Adjusts file copy APIs so
CopyDirRecursiveremains dry-run-aware, whileCopyDirRecursiveCrossFSis unconditional (so embedded defaults extraction always happens). - Removes
DryRunnableplumb-through from project config loading and default config extraction to avoid dry-run short-circuiting. - Adds/updates unit tests for dry-run directory-copy suppression and for loading configs with embedded defaults.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/utils/fileutils/copy.go | Refactors copy helpers; moves dry-run behavior to CopyDirRecursive and makes cross-FS recursive copy unconditional. |
| internal/utils/fileutils/copy_test.go | Adds a test asserting recursive dir copy is suppressed in dry-run mode. |
| internal/projectgen/projectgen_test.go | Updates LoadProjectConfig call sites to match the new signature. |
| internal/projectconfig/config.go | Removes DryRunnable dependency and always extracts embedded defaults when enabled. |
| internal/projectconfig/config_test.go | Adds a default-config loading test and updates existing tests for the new LoadProjectConfig signature. |
| internal/app/azldev/app.go | Removes dryRunnable plumbing into config loading. |
| defaultconfigs/defaultconfigs.go | Updates CopyTo signature and call path to use unconditional cross-FS recursive copy. |
| err = fileutils.CopyDirRecursiveCrossFS( | ||
| dryRunnable, | ||
| sourceFS, embedfsRootDir, | ||
| fs, destPath, | ||
| fileutils.CopyDirOptions{}, |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@copilot There are two review comments, please have a look at them. I applied the suggested fix for one, please triage and consider fixing the 2nd. |
liunan-ms
left a comment
liunan-ms
left a comment
There was a problem hiding this comment.
LGTM, approved 👍
Is there an open bug for this that can be linked here?
|
Description
Dry-run skipped extracting embedded defaults into azldev’s temporary directory, causing project configuration loading to fail before command execution.
Changes
CopyDirRecursivedry-run-aware.CopyDirRecursiveCrossFSunconditional, matching its contract.