Improvements to param and path systems, fix #239 #240
Merged
Conversation
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
…ation Introduced scopes to suppress parameter resolution, enforce default values, or override parameter sources temporarily. Refactored related logic to improve maintainability. Updated `BuildResolver` and tests to integrate with these changes.
Previously, `TypeUtil.TryConvert` did not account for cases where the type is `object`. Added logic to return the string value when the type is `object`, ensuring proper conversion behavior.
Integrated `IParamService` in `GithubWorkflowWriter` to support disposable scopes for enhanced parameter resolution. This ensures default values are properly handled during workflow generation. Also created tests to verify functionality.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the parameter and path systems to improve their design and fix circular dependency issues (issue #239). The changes include simplifying the path provider API, adding new parameter resolution scopes, and fixing minor UI/display issues.
Key changes:
- Refactored
IPathProviderinterface to remove the circular dependency-pronelocatorparameter, replacing it with direct depth-based recursion detection - Added
CreateDefaultValuesOnlyScope()andCreateOverrideSourcesScope()methods toIParamServicefor better control over parameter resolution behavior - Improved console output formatting for sub-10ms target durations and workflow generation stability
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| DecSm.Atom/Util/TypeUtil.cs | Added support for converting string values to object type |
| DecSm.Atom/Reports/ConsoleOutcomeReportWriter.cs | Improved duration display for very fast targets (<10ms) and added spacing |
| DecSm.Atom/Paths/PathProvider.cs | Deleted - split into separate interface and implementation files |
| DecSm.Atom/Paths/IPathProvider.cs | Simplified interface by removing locator parameter and renaming Locate to GetPath |
| DecSm.Atom/Paths/FunctionPathProvider.cs | New concrete implementation replacing old PathProvider class |
| DecSm.Atom/Paths/IPathLocator.cs | Renamed interface from IFileMarker for clarity |
| DecSm.Atom/Paths/AtomPaths.cs | Added new ProvidePath overload accepting IAtomFileSystem parameter |
| DecSm.Atom/Paths/AtomFileSystem.cs | Added depth-based circular dependency detection, simplified path resolution logic |
| DecSm.Atom/Params/ParamService.cs | Added new scope types for default-only and source override scenarios |
| DecSm.Atom/Build/Definition/MinimalBuildDefinition.cs | Removed SuppressParamResolution property (moved to service) |
| DecSm.Atom/Build/Definition/IBuildDefinition.cs | Removed param suppression API (moved to service) |
| DecSm.Atom/Build/BuildResolver.cs | Updated to use new IParamService.CreateDefaultValuesOnlyScope() |
| DecSm.Atom/Build/BuildExecutor.cs | Removed redundant console output |
| DecSm.Atom.Tests/ClassTests/Logging/MaskingAnsiConsoleOutputTests.cs | Updated stub to implement new IParamService methods |
| DecSm.Atom.Tests/ClassTests/Build/DefaultBuildVersionProviderTests.cs | Removed obsolete ClearCache() call |
| DecSm.Atom.Tests/ClassTests/Build/BuildResolverTests.cs | Updated tests to pass IParamService to BuildResolver |
| DecSm.Atom.Tests/BuildTests/FileSystem/FileSystemTests.cs | Updated to use FunctionPathProvider instead of PathProvider |
| DecSm.Atom.SourceGenerators/GenerateSolutionModelSourceGenerator.cs | Updated generated code to use IPathLocator instead of IFileMarker |
| DecSm.Atom.Module.GithubWorkflows/IGithubWorkflows.cs | Updated path provider to use new API with IAtomFileSystem |
| DecSm.Atom.Module.GithubWorkflows/Generation/GithubWorkflowWriter.cs | Added scopes for stable workflow generation regardless of parameter values |
| DecSm.Atom.Module.GithubWorkflows.Tests/Workflows/WorkflowTests.cs | Added test verifying workflow generation stability |
| DecSm.Atom.Module.GithubWorkflows.Tests/Workflows/ManualInputStabilityBuild.cs | New test fixture for workflow stability testing |
| DecSm.Atom.Module.DevopsWorkflows/IDevopsWorkflows.cs | Updated path provider to use new API with IAtomFileSystem |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #239