Skip to content

Feat/cf 2275 analyze api#199

Open
og-pixel wants to merge 2 commits intomainfrom
feat/CF-2275-analyze-api
Open

Feat/cf 2275 analyze api#199
og-pixel wants to merge 2 commits intomainfrom
feat/CF-2275-analyze-api

Conversation

@og-pixel
Copy link
Copy Markdown

Extra check when running analyze. If tool config does not exist in codacy config, it will check root first before attempting to create defauls.

If config does not exist in .codacy/tools-configs
it will first check if the root of the project
contains any configurations.
Copilot AI review requested due to automatic review settings March 25, 2026 14:30
@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Mar 25, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 4 complexity . 0 duplication
Metric Results
Complexity 4
Duplication 0

View in Codacy

🟢 Coverage 100.00% diff coverage · +0.09% coverage variation
Metric Results
Coverage variation +0.09% coverage variation (-0.50%)
Diff coverage 100.00% diff coverage (50.00%)

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (5d0b1ca) 5990 1303 21.75%
Head commit (926b904) 5997 (+7) 1310 (+7) 21.84% (+0.09%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#199) 9 9 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes. Give us feedback

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a safeguard to analyze so that when a tool config is missing from .codacy/tools-configs/, the CLI checks for an existing user config in the repository root before generating a default.

Changes:

  • Update checkIfConfigExistsAndIsNeeded to skip default config creation when a repo-root config file is present.
  • Extend TestCheckIfConfigExistsAndIsNeeded to cover repo-root config scenarios and assert whether a tools-configs file is created/present.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
cmd/analyze.go Adds repo-root config detection to avoid generating defaults when the user already has a tool config.
cmd/analyze_test.go Expands unit tests to cover repo-root config presence and expectations around tools-configs file generation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@og-pixel og-pixel force-pushed the feat/CF-2275-analyze-api branch from 5ca245a to 926b904 Compare March 25, 2026 14:48
Copy link
Copy Markdown

@codacy-production codacy-production bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

The PR successfully implements the logic to detect tool configurations in the repository root, preventing unintended overrides when the CLI falls back to default settings. Codacy reports the PR as 'up to standards' with 100% diff coverage; however, there are notable maintainability regressions.

Key areas requiring attention before merging:

  • Complexity Regressions: The logic within TestCheckIfConfigExistsAndIsNeeded has reached a cyclomatic complexity of 13, making the test difficult to maintain.
  • Function Bloat: runToolByName now takes 9 parameters, which is a significant anti-pattern in Go and should be refactored to use a configuration struct.
  • Implementation Discrepancy: The code checks .codacy/tools-configs/ before the repository root, which contradicts the 'root first' description in the PR. While functionally sound, the documentation should be updated to match the implementation.

About this PR

  • The Jira ticket identifies the root cause of this issue as the 'analyze' command running without an expected API token. While this PR effectively mitigates the resulting configuration override, it does not address why the token is missing in the first place. Consider if additional logging or validation is needed for the token lifecycle.
  • The implementation checks '.codacy/tools-configs/' before the repository root. This is the inverse of the PR description which mentions 'root first'. Please ensure the description or documentation is updated to reflect that the CLI prioritizes the tool-configs directory for overrides.
3 comments outside of the diff
cmd/analyze.go

line 318 🟡 MEDIUM RISK
The complexity of this function is increasing due to nested filesystem checks. Consider extracting a locateConfig(toolName string) string helper to find the config file path and a provisionConfig(...) helper for the creation logic to decouple these responsibilities.

line 368 🟡 MEDIUM RISK
Suggestion: This function signature is becoming difficult to manage with 9 parameters. Grouping these into a configuration struct (e.g., ToolRunOptions) would make the API cleaner, more readable, and easier to extend without breaking existing calls.

cmd/analyze_test.go

line 414 🔴 HIGH RISK
The logic within this test's t.Run closure has grown to a complexity of 13. This is primarily due to the integrated setup of temporary directories, configuration mocking, and file writing. Extracting these steps into helper functions will improve test clarity and maintainability. Consider extracting the environment setup and result verification logic.

Test suggestions

  • Verify that the CLI detects a tool configuration in the repository root and skips default creation in both local and remote modes.
  • Verify that the CLI correctly identifies an existing configuration in '.codacy/tools-configs' and proceeds without creating a duplicate or checking the root.
  • Verify that the CLI creates a default configuration in '.codacy/tools-configs' when no config exists in either location and an API token is provided.
  • Verify that no configuration is created if files are missing in both locations and no API token is provided in remote mode.

🗒️ Improve review quality by adding custom instructions

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.

3 participants