Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

67 java #44

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

67 java #44

wants to merge 3 commits into from

Conversation

pataluc
Copy link

@pataluc pataluc commented May 30, 2024

Rule now only reports when i++ is in statement.

Issue #4

@cyChop
Copy link

cyChop commented May 30, 2024

Quid d'un test case (compliant) int a = 2 + counter++ ?
(Normalement couvert par bar61(2 + counter++), mais ce cas était également déjà couvert par bar61(counter++).)

@pataluc
Copy link
Author

pataluc commented May 30, 2024

Quid d'un test case (compliant) int a = 2 + counter++ ? (Normalement couvert par bar61(2 + counter++), mais ce cas était également déjà couvert par bar61(counter++).)

après discussion avec @dedece35 hier, au final on ne cherche que les cas ou le parent de i++ est une Kind.EXPRESSION_STATEMENT, parce que les autres cas sont soit volontaires, soit des code smells déjà reporté par les règles Sonar hors ecocode. du coup je ne suis pas sûr que cela apporte qqchose de rajouter ce cas (même si c'est très simple de l'ajouter)

Copy link

sonarcloud bot commented May 31, 2024

@dedece35
Copy link
Member

Quid d'un test case (compliant) int a = 2 + counter++ ? (Normalement couvert par bar61(2 + counter++), mais ce cas était également déjà couvert par bar61(counter++).)

après discussion avec @dedece35 hier, au final on ne cherche que les cas ou le parent de i++ est une Kind.EXPRESSION_STATEMENT, parce que les autres cas sont soit volontaires, soit des code smells déjà reporté par les règles Sonar hors ecocode. du coup je ne suis pas sûr que cela apporte qqchose de rajouter ce cas (même si c'est très simple de l'ajouter)

Hello ... merci pour la précision ... mais peut-être qu'on s'est mal compris, perso, je serai aussi pour checker le uses case de @cyChop même si je pense qu'il est déjà couvert (ça ne coute rien de rajouter un cas de test dans les fichiers de tests et voir ce qui se passe en TU, puis en test env reel et vérifier si SonarQube remonte déjà un code smell natif sur ces cas là)
Donc, je préfère tout de meme rajouter du check et du test pour vérifier please.

Copy link
Member

@dedece35 dedece35 left a comment

Choose a reason for hiding this comment

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

cf les commentaires ci-dessus

Copy link

github-actions bot commented Jul 1, 2024

This PR has been automatically marked as stale because it has no activity for 30 days.
Please add a comment if you want to keep the issue open. Thank you for your contributions!

@github-actions github-actions bot added the stale label Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants