Skip to content

Improve lizard config behaviour#122

Closed
franciscoovazevedo wants to merge 1 commit intopluto-1380from
improve-lizard-confi-behaviour
Closed

Improve lizard config behaviour#122
franciscoovazevedo wants to merge 1 commit intopluto-1380from
improve-lizard-confi-behaviour

Conversation

@franciscoovazevedo
Copy link
Copy Markdown
Contributor

based on #119

we were fetching codacy default patterns when no patterns existed we should just keep as it is defined in the repo.

@codacy-production
Copy link
Copy Markdown

codacy-production Bot commented May 15, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.79% 75.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (9b41044) 4637 1424 30.71%
Head commit (97f16ab) 4628 (-9) 1458 (+34) 31.50% (+0.79%)

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 (#122) 4 3 75.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%

See your quality gate settings    Change summary preferences

Comment thread cmd/analyze.go
if err != nil {
return fmt.Errorf("failed to fetch default patterns: %v", err)
}
patterns = []domain.PatternDefinition{}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't understand this change. On the analysis standpoint, the tool should just rely on the patterns file and that is that.

I would leave this as it was or update this tool to actually use the tools.ConfigFileExists( as the other tools use. As this at most should be a failsafe anyway.

tools.ConfigFileExists check for configuration file both generated from the API during init, or if it exists on the root of the repo. So if we initialize from the UI with no patterns, we should have a file created with no rules, and don't even hit this path

Comment thread cmd/init.go
patterns := make([]domain.PatternDefinition, len(patternConfiguration))
for i, pattern := range patternConfiguration {
patterns[i] = pattern.PatternDefinition

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These changes are a bit strange for me, if we check the ifs for other tools, there are things like

	case DartAnalyzer:
		if len(patternConfiguration) > 0 {
			err := createDartAnalyzerConfigFile(patternConfiguration, toolsConfigDir)
			if err != nil {
				return fmt.Errorf("failed to create Dart Analyzer config: %v", err)
			}
			fmt.Println("Dart configuration created based on Codacy settings")
		}
	case Semgrep:
		if len(patternConfiguration) > 0 {
			err := createSemgrepConfigFile(patternConfiguration, toolsConfigDir)
			if err != nil {
				return fmt.Errorf("failed to create Semgrep config: %v", err)
			}
			fmt.Println("Semgrep configuration created based on Codacy settings")
		}
	case Lizard:
		createLizardConfigFile(toolsConfigDir, patternConfiguration)
	}

But for Lizard you only call this method, but then you remove the if?

@franciscoovazevedo franciscoovazevedo force-pushed the improve-lizard-confi-behaviour branch from 77d1012 to 97f16ab Compare May 19, 2025 10:56
@franciscoovazevedo
Copy link
Copy Markdown
Contributor Author

#127 approached here

@franciscoovazevedo
Copy link
Copy Markdown
Contributor Author

#127 fixed this.

@alerizzo alerizzo deleted the improve-lizard-confi-behaviour branch June 3, 2025 09:45
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.

2 participants