iluwatar/java-design-patterns

SonarCloud reports issues

iluwatar opened this issue · 43 comments

iluwatar commented

The project is using SonarCloud static code analysis. The latest results are showing that it has found several blockers and critical severity code smells (major, minor, and info level we ignore). In this task, those are fixed or marked as false positives. To mark false positives, SuppressWarnings annotation should be used in code as explained in the linked documentation.

Links:
Blocker and critical severity code smells in SonarCloud
Sonar false positives

Note: When Submitting a PR please provide a hyperlinked list of all the issues that you are issuing a fix for. See this example

ykayacan commented

Can I take this issue?

  • 🎉1
iluwatar commented

Sure @ykayacan

  • 🚀2
iluwatar commented

This issue is free for taking again.

  • 👀1
ImLynxie commented

I would like to take this issue.

ImLynxie commented

Through out checking the files causing issues on SonarCloud, I've encountered test cases that are not properly named to the functionality that they are to be tested against.

Example`

In-correct naming convention of a test method.

@Test void test() throws Exception { App.main(new String[]{}); }

Correct naming convention of a test method.

@Test public void shouldExecuteAppWithoutException() { assertDoesNotThrow(() -> App.main(null)); }

A test method should describe what its testing directly in the naming, I will rename the test cases that I find throughout the refactoring process to match proper convention rules, if the test case is understandable as to what it's testing, but I would encourage that these test cases be looked at and properly named for other contributors making changes to certain files that the test cases test against, otherwise it causes a lot of confusion as to whether said contributor is testing the right thing.

  • 👍1
iluwatar commented

Ok @ToxicDreamz, assigned to you. I agree with your comment regarding test naming.

ImLynxie commented

I've already finished with the issue.

Request to run another SonarCloud report after merging, to see what else remains, and whether or not they can be modified so issues do not come up again.

anuragagarwal561994 commented

@iluwatar happy to be back again :)
Can I help with this issue?

iluwatar commented

Ok, assigned @anuragagarwal561994 and @daniel-augusto. Thanks guys!

kanwarpreet25 commented

@iluwatar Can I help with this issue?

iluwatar commented

Sure if @anuragagarwal561994 is no longer working on it?

anuragagarwal561994 commented

No @iluwatar I didn't get time to look into this, you can re-assign

kanwarpreet25 commented

@iluwatar I will start working on it

iluwatar commented

No problem @anuragagarwal561994
@kanwarpreet25 you're good to go

solidnetwork-xyz commented

@iluwatar can I help with this and coordinate with @kanwarpreet25 @daniel-augusto ?

solidnetwork-xyz commented

@kanwarpreet25 has no public commits since Jul 28, 2019, f7e22a1
@daniel-augusto has no public commits to this repo since Oct 11, 2020 3a72abb

Are you guys working on this know?

iluwatar commented

Thanks for helping out @joseosuna-engineer. I'm looking forward to your pull request!

solidnetwork-xyz commented

Thank you. I'm going to begin with this.

solidnetwork-xyz commented

@iluwatar @ohbus
I have added this project to my own CircleCI and SonarCloud accounts to run SonarCloud before create a pull request.

My accounts:
image

I changed two config files, 😱 .circleci/config.yml and pom.xml (my fork, another branch)
I'm sure @iluwatar @ohbus that we can find a better approach using environment vars or some characteristic of
CircleCI 2.1 Youtube

My strategy is:

  • to use this branch A only in my fork.
  • to create another branch B to commit my code.
  • to merge B to A and run CircleCI + SonarCloud
  • when code is okay create a pull request from B (that has original config files) to master

image

image

iluwatar commented

I think the general approach presented above is going to work, but as pointed out there are chances to streamline the workflow. @joseosuna-engineer feel free to create another issue where you point out the exact spots we should change.

  • 👍1
solidnetwork-xyz commented

@iluwatar I have created issue 1697

solidnetwork-xyz commented

The project is using SonarCloud static code analysis. The latest results are showing that it has found several blocker and critical severity code smells (major, minor and info level we ignore). In this task those are fixed or marked as false positives. To mark false positives, SuppressWarnings annotation should be used in code as explained in the linked documentation.

Links:
Blocker and critical severity code smells in SonarCloud
Sonar false positives

@iluwatar I want to ask you some things:
1.- This issue (1012) is not about all sonarcloud issues. is it?
2.- major, minor and info issues are not included here
3.- it is only about (blocker and critical) + code smell issues

image

4.- Do you want I add @java.lang.SuppressWarnings("squid:some-number") annotation for all in question number 3? or Do I have to do something like ykayacan: suppress false positives (test by example) and fix number 3 issues?

iluwatar commented

@joseosuna-engineer

  1. No, it's about blocker and critical level issues only
  2. Yes
  3. Yes
  4. For false positives we can suppress the warnings as you suggested
  • 👍1
solidnetwork-xyz commented

@samuelpsouza Yes. Feel free to colaborate. @iluwatar I'm so sorry. I'm studying for a couple of certification test.

  • 👍2
ohbus commented

@samuelpsouza Yes. Feel free to collaborate. @iluwatar I'm so sorry. I'm studying for a couple of certification tests.

Please @joseosuna-engineer, no need for an apology here.

You can mention a timeline for when you will be able to make the contributions or let us know in Gitter about the same as well.

And all the very best for your examinations! May you ace them ⭐

  • 👍2
ohbus commented

Can I help to fix this issue? Has anyone solved this one? https://sonarcloud.io/project/issues?id=iluwatar_java-design-patterns&open=AW3G0WaAwB6UiZzQNqwC&resolved=false&severities=BLOCKER

Thank you so much for your interest in our project @samuelpsouza through #1784

parulagg27 commented

I would like to work on this issue and resolve some of the code smells/ critical issues in Sonar. Can I take this up? @iluwatar

iluwatar commented

Sure @parulagg27, go ahead

iluwatar commented

We need assistance to resolve the remaining blocker and critical level SonarCloud issues

interactwithankush commented

I have resolved 1 blocker & 5 critical Sonar issues using this PR. Kindly review and merge.
Screenshot 2021-10-30 at 11 27 15 PM
Screenshot 2021-10-30 at 11 26 50 PM

  • 👍1
interactwithankush commented

  • Resolved 1 blocker & 5 critical Sonar issues

  • Old sonar report :- here

  • Code Cov: Added an extensive test cases for Commander pattern.

  • PR Quality Gate is also green

@iluwatar @ohbus
Kindly review and merge this PR

ShivanshCharak commented

hey can i work on this issue :)

iluwatar commented

Ok, go ahead @ShivanshCharak

ShivanshCharak commented

Raised a pr for the following Critical code smell
#1960

please also review
a minor code smell
#1959 (comment)

S7sRuss commented

Can I help?

iluwatar commented

Ok @S7sRuss, thanks for the help