Consolidate strict mode validation into strict_mode_validation.go#3492
Consolidate strict mode validation into strict_mode_validation.go#3492
Conversation
- Rename validation_strict_mode.go to strict_mode_validation.go for consistency - Move validateStrictMode orchestrator from strict_mode.go to validation file - Delete strict_mode.go as it's now redundant - Update documentation references in validation.go, CONTRIBUTING.md, and specs/validation-architecture.md - All tests pass successfully Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR consolidates strict mode validation logic by merging two files (strict_mode.go and validation_strict_mode.go) into a single strict_mode_validation.go file. The changes include:
- Moving the
validateStrictMode()orchestrator function intostrict_mode_validation.go - Deleting the now-redundant
strict_mode.gofile - Updating documentation references throughout to reflect the new file structure
However, the documentation also introduces references to pip_validation.go, npm_validation.go, and docker_validation.go which don't exist in the codebase.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
specs/validation-architecture.md |
Updates documentation to reference the consolidated strict_mode_validation.go file and mentions validation files with *_validation.go suffix |
pkg/workflow/validation.go |
Updates file header comments to reference strict_mode_validation.go and introduces references to non-existent *_validation.go files |
pkg/workflow/strict_mode_validation.go |
Adds the validateStrictMode() orchestrator function from deleted strict_mode.go with enhanced documentation |
pkg/workflow/strict_mode.go |
File deleted - functionality moved to strict_mode_validation.go |
CONTRIBUTING.md |
Updates validation architecture documentation with references to non-existent *_validation.go files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // - pip_validation.go: Python package validation | ||
| // - npm_validation.go: NPM package validation | ||
| // - docker_validation.go: Docker image validation |
There was a problem hiding this comment.
The files pip_validation.go, npm_validation.go, and docker_validation.go don't exist in the codebase. The actual files are pip.go, npm.go, and docker.go. These references should be corrected to match the actual filenames, or if a renaming is planned, those files should be renamed in the same PR.
| // - pip_validation.go: Python package validation | |
| // - npm_validation.go: NPM package validation | |
| // - docker_validation.go: Docker image validation | |
| // - pip.go: Python package validation | |
| // - npm.go: NPM package validation | |
| // - docker.go: Docker image validation |
| - `pip_validation.go` - Python package validation | ||
| - `npm_validation.go` - NPM package validation | ||
| - `docker_validation.go` - Docker image validation |
There was a problem hiding this comment.
The files pip_validation.go, npm_validation.go, and docker_validation.go don't exist in the codebase. The actual files containing validation logic are pip_validation.go (exists), npm_validation.go (exists), and docker_validation.go (exists) for validation functions, but the extraction and some validation is in pip.go, npm.go, and docker.go. This documentation should reference the actual existing files.
| - `pip_validation.go` - Python package validation | |
| - `npm_validation.go` - NPM package validation | |
| - `docker_validation.go` - Docker image validation | |
| - `pip.go` - Python package validation | |
| - `npm.go` - NPM package validation | |
| - `docker.go` - Docker image validation |
| **Architecture**: The strict mode validation is split across two files for better organization: | ||
| - `strict_mode.go` contains the main orchestrator that coordinates validation | ||
| - `validation_strict_mode.go` contains the individual validation function implementations | ||
| **Architecture**: All strict mode validation logic is consolidated in a single file following the `*_validation.go` naming pattern used throughout the codebase |
There was a problem hiding this comment.
The claim that the *_validation.go naming pattern is "used throughout the codebase" is misleading. While pip_validation.go, npm_validation.go, and docker_validation.go exist, many other domain-specific files like pip.go, npm.go, docker.go, expression_safety.go, engine.go, mcp-config.go, and template.go don't follow this pattern. Consider revising to say "following the *_validation.go naming convention" without claiming it's used throughout.
| **Architecture**: All strict mode validation logic is consolidated in a single file following the `*_validation.go` naming pattern used throughout the codebase | |
| **Architecture**: All strict mode validation logic is consolidated in a single file following the `*_validation.go` naming convention |
Refactors strict mode validation to follow the established
*_validation.gonaming convention used throughout the codebase (e.g.,docker_validation.go,npm_validation.go,pip_validation.go).Changes
validation_strict_mode.go→strict_mode_validation.gofor consistency with naming patternvalidateStrictMode()orchestrator fromstrict_mode.gointo validation filestrict_mode.go(now redundant - contained only orchestrator function)validation.go,CONTRIBUTING.md, andspecs/validation-architecture.mdStructure
All strict mode validation logic now lives in a single 161-line file:
Resolves #3478 (Priority 2 from semantic function clustering analysis).
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.