Skip to content

Conversation

@Pankraz76 Pankraz76 changed the title add test assertion to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") resolve unused assertion to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") May 21, 2025
@Pankraz76 Pankraz76 changed the title resolve unused assertion to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") resolve unused assignment to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") May 21, 2025
@Pankraz76 Pankraz76 marked this pull request as ready for review May 21, 2025 12:20
@Pankraz76 Pankraz76 force-pushed the fix-test-collector branch from 9906073 to d1c7001 Compare May 21, 2025 18:44
@Pankraz76 Pankraz76 requested review from desruisseaux and gnodet May 21, 2025 18:45
@Pankraz76 Pankraz76 changed the title resolve unused assignment to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") improve test to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") May 21, 2025
@Pankraz76 Pankraz76 changed the title improve test to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") add test assertion to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") May 21, 2025
@Pankraz76
Copy link
Contributor Author

whats he issue with this one?

assuming its the same solution for common problem.

@Pankraz76 Pankraz76 changed the title add test assertion to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") chore: add test assertion to resolve @SuppressWarnings("checkstyle:UnusedLocalVariable") May 22, 2025
@desruisseaux
Copy link
Contributor

This pull request provides small changes addressing exactly the discussion point, while #2365 is a larger set of changes which includes questionable changes (e.g. the introduction of the assertHeader method).

Addressing all PMD warnings should not be a goal. Many of them are harmless, and sometime complying to the rule make the code less understandable. Some warnings are indeed real issues (e.g. the while (vector.isEmpty()) loop in another PR), but the evaluation is on a case-by-case basis.

@Pankraz76
Copy link
Contributor Author

which includes questionable changes (e.g. the introduction of the assertHeader method).

yes not applied SOC, sorry.

Addressing all PMD warnings should not be a goal.

kind of yes. I will check the most important.

@Pankraz76 Pankraz76 force-pushed the fix-test-collector branch from d1c7001 to 45c0472 Compare May 28, 2025 17:39
@Pankraz76 Pankraz76 force-pushed the fix-test-collector branch from 45c0472 to f63e376 Compare May 28, 2025 17:43
@Pankraz76 Pankraz76 requested a review from desruisseaux May 28, 2025 17:43
@Pankraz76
Copy link
Contributor Author

Pankraz76 commented May 31, 2025

this could be merge as approved and just incremented the test little further.
merci.

@slachiewicz slachiewicz closed this Jun 9, 2025
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.

4 participants