Skip to content

Conversation

@franciscoovazevedo
Copy link
Contributor

No description provided.

@codacy-production
Copy link

codacy-production bot commented Apr 3, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+2.32% 69.17%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (47cfc56) 2408 598 24.83%
Head commit (c4a521f) 2541 (+133) 690 (+92) 27.15% (+2.32%)

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 (#53) 133 92 69.17%

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

Copy link
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

This PR introduces functionality to parse API v3 pattern JSON data into a Pylint RC configuration file. Key changes include:

  • Adding new type definitions for Pylint pattern configurations in tools/types/pylint_types.go.
  • Implementing a mapping function to derive section names for Pylint parameters in tools/pylint/src/pylintParameterSectionMap.go.
  • Providing functions to parse the JSON data, generate a pylintrc file, and group parameters by section in tools/pylint/src/pylintConfigParser.go.

Reviewed Changes

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

File Description
tools/types/pylint_types.go Added new type definitions for Pylint pattern configurations
tools/pylint/src/pylintParameterSectionMap.go Introduced mapping for parameter section names to support configuration
tools/pylint/src/pylintConfigParser.go Added JSON parsing, RC file generation, and grouping logic for patterns
Comments suppressed due to low confidence (1)

tools/types/pylint_types.go:5

  • [nitpick] Consider renaming field 'Id' to 'ID' to align with Go naming conventions.
Id                string

Copy link
Contributor

@machadoit machadoit left a comment

Choose a reason for hiding this comment

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

I guess you mention apiv3 on the title, but we still rely on the v2 here right? Regardless, semms that it should do the trick!

@franciscoovazevedo franciscoovazevedo changed the title feature: parse apiv3 patterns jsonData into pylintrc type file - PLUTO-1358 feature: parse apiv2 patterns jsonData into pylintrc type file - PLUTO-1358 Apr 7, 2025
package pylint

// DefaultPatterns contains the list of default Pylint patterns
var DefaultPatterns = []string{
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is the thing that we can change to get from the API, but I am fine moving forward with this on this scope

var testData = `{
"patterns": [
{
"patternDefinition": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe as a test, we can have here less patterns?

Copy link
Contributor

@machadoit machadoit left a comment

Choose a reason for hiding this comment

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

LGTM!

fixed test

updated to use the ight v2 endpoint

creating default pylint if no configuration found
@franciscoovazevedo franciscoovazevedo merged commit d0b14b3 into main Apr 9, 2025
7 checks passed
@alerizzo alerizzo deleted the pylint-config-parser 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.

3 participants