Skip to content

Conversation

@joke1196
Copy link
Contributor

@joke1196 joke1196 commented Sep 8, 2025

SCANPY-214

Part of

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod bot changed the title Updated doc SCANPY-215 Updated doc Sep 8, 2025
@joke1196 joke1196 changed the title SCANPY-215 Updated doc SCANPY-214: Change property name --sonar-cpd-python-minimum-tokens to --sonar-cpd-py-minimum-tokens Sep 8, 2025
@joke1196 joke1196 requested a review from Seppli11 September 8, 2025 09:45
Copy link
Contributor

@Seppli11 Seppli11 left a comment

Choose a reason for hiding this comment

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

The change itself looks good. However, this would be a breaking change.
As such, I think it would make sense to keep both --sonar-cpd-py-minimum-lines and --sonar-cpd-python-minimum-lines (the same applies for the minimum tokens).

This would still be a breaking change for the environment variable parsing, but I feel like we could live with that

@joke1196 joke1196 requested a review from Seppli11 September 8, 2025 12:28
Copy link
Contributor

@Seppli11 Seppli11 left a comment

Choose a reason for hiding this comment

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

LGTM! thanks for the quick fix.

I think we don't need the -D... args, since they just act as documentation.

(Sorry, I probably could have been a bit clearer in my review)

"--sonar-cpd-py-minimum-lines",
"-Dsonar.cpd.py.minimumLines",
"--sonar-cpd-python-minimum-lines",
"-Dsonar.cpd.python.minimumLines",
Copy link
Contributor

Choose a reason for hiding this comment

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

The -Dsonar.cpd.python.minimumLines we shouldn't need, since any non-consumed parameter beginning with -D... will be passed along.

Same applies for minimum tokens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should keep them as we will pass the property to the correct sonar.cpd.py....

Copy link
Contributor

Choose a reason for hiding this comment

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

True. I think the PR can be merged then

@sonarqube-next
Copy link

sonarqube-next bot commented Sep 8, 2025

@joke1196 joke1196 merged commit 0f857a6 into master Sep 8, 2025
17 checks passed
@joke1196 joke1196 deleted the SCANPY-214 branch September 8, 2025 14:13
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