Skip to content

Sonar report - fix blocker and critical ones #1899

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 34 commits into from
Dec 16, 2021
Merged

Sonar report - fix blocker and critical ones #1899

merged 34 commits into from
Dec 16, 2021

Conversation

interactwithankush
Copy link
Contributor

Change description :-

  • fixed Sonar blocker and critical issues related to duplicate String literals
  • Refer to the issue that it solves, if available

Pull request description

  • fixed Sonar blocker and critical issues related to duplicate String literals
  • removed unnecessary empty default constructor

@interactwithankush interactwithankush mentioned this pull request Oct 30, 2021
Closed
@interactwithankush
Copy link
Contributor Author

@iluwatar @ohbus Kindly review

@interactwithankush
Copy link
Contributor Author

Please put review comments as needed
@iluwatar @ohbus

@interactwithankush
Copy link
Contributor Author

@ohbus @iluwatar . I am done with my changes. Kindly review.

@ohbus ohbus self-assigned this Nov 4, 2021
ohbus
ohbus suggested changes Nov 4, 2021
View reviewed changes
Copy link
Contributor

@ohbus ohbus left a comment

Choose a reason for hiding this comment

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

Thank you so much for your interest in our project.

I would request if you could please increase the Code COverage to at least 80%

Thanks!

@interactwithankush
Copy link
Contributor Author

Hi @ohbus . Thanks for allowing me to contribute in this well kept repo to help serve the java community.
While removing some blocker & critical bugs reported by sonar, I have also tried adding a Test class to cover as much as I could. The original issue was about fixing sonar bugs. And while I fixed those, sonar somehow annotated those existing lines as new ones, and is expecting to have test cases for all of them. I have tried to bring it up to 50%. I guess that is the best I could do for this PR. I can pick up test coverage as a separate PR, and while not digressing from the objective of this PR. Do let know if there is any specific review for this PR. Thanks.

@interactwithankush
Copy link
Contributor Author

@ohbus Is there any review comment I can help with before merging this PR?

ohbus
ohbus reviewed Nov 12, 2021
View reviewed changes
Copy link
Contributor

@ohbus ohbus left a comment

Choose a reason for hiding this comment

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

So far so good. Thank you so much for the improvement this looks very promising and you have really put a lot of effort into this.

I would still want you to look into this code smell as it has come up and would really appreciate it if you could please help resolve this.

@interactwithankush
Copy link
Contributor Author

Hi @ohbus .
Last code smell is cleaned in latest. Thanks.

@interactwithankush
Copy link
Contributor Author

Hi @ohbus
Just curious about when could we expect this PR to get merged?

@interactwithankush
Copy link
Contributor Author

I guess the build is failing because "SonarCloud is under maintenance"
image

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

84.3% 84.3% Coverage
0.0% 0.0% Duplication

@interactwithankush
Copy link
Contributor Author

Hi @ohbus . The changes you asked for have been incorporated.
Please add if there are any more comments on this PR.

@interactwithankush
Copy link
Contributor Author

@ohbus
All requested changes are completed. Kindly review the PR.

@interactwithankush
Copy link
Contributor Author

Hi @ohbus @iluwatar
Being a time bound activity, could you please complete the review process.

@interactwithankush
Copy link
Contributor Author

@ohbus @iluwatar Waiting for review & merge :)

@ohbus
Copy link
Contributor

ohbus commented Nov 15, 2021

This is Open Source and people here contribute out of their passion during their free time for which they are almost never rewarded.

Please do not mention us repeatedly for the same thing which spams our notification for something which we have in our mind when you pushed your commit.

Currently we may not have the bandwidth to accommodate these requests, as maintaiers might be busy with something else.

Please have some patience, we will do our due diligence in due course of time.

@interactwithankush
Copy link
Contributor Author

Hi @ohbus
Just a peaceful reminder. I do not intend to spam. Thx

@interactwithankush
Copy link
Contributor Author

@ohbus Hope this long pending PR is taken up soon 👍

ohbus
ohbus approved these changes Dec 16, 2021
View reviewed changes
Copy link
Contributor

@ohbus ohbus left a comment

Choose a reason for hiding this comment

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

Thank you so much for your efforts @interactwithankush

@ohbus ohbus merged commit 600227d into iluwatar:master Dec 16, 2021
@ohbus
Copy link
Contributor

ohbus commented Dec 16, 2021

@all-contributors please add @interactwithankush for code

Thank you for your patience. This has been long due.

@allcontributors allcontributors bot mentioned this pull request Dec 16, 2021
Merged
@allcontributors
Copy link
Contributor

@ohbus

I've put up a pull request to add @interactwithankush! 🎉

@iluwatar iluwatar added this to the 1.26.0 milestone Dec 19, 2021
RobbiNespu pushed a commit to RobbiNespu/java-design-patterns that referenced this pull request Mar 1, 2022
@interactwithankush @ohbus @RobbiNespu
fix: Sonar report - fix blocker and critical ones (iluwatar#1899)
5e0919d 
* update SpatialPartitionBubbles - fix Sonar blocker issue

* fix Sonar critical issue - Define constant instead of duplicating the literal

* fix Sonar critical issue - remove unnecessary default constructor

* fix Sonar critical issue - Define constant instead of duplicating the literal

* fix Sonar critical issue - Define constant instead of duplicating the literal

* fix Sonar critical issue - Define constant instead of duplicating the literal

* fix Sonar critical issue - fix checkstyle issue

* fix Sonar critical issue - fix code smells

* fix Sonar critical issue - fix code smells

* fix Sonar critical issue - fix code smells

* fix sonarbugs - adding test cases for Commander class

* sonar fix - add assert commands in CommanderTest

* sonar fix - add test cases for CommanderTest

* sonar fix - add test cases for CommanderTest

* sonar fix - add test cases for CommanderTest

* sonar fix - add test cases for CommanderTest

* sonar fix - add test cases for CommanderTest

* sonar fix - add test cases for CommanderTest

* sonar fix - add test cases for CommanderTest

* sonar bug fix & test cases

* sonar bug fix & test cases

* sonar bug fix & test cases

* sonar bug fix & test cases

* sonar bug fix & test cases

* Revert "sonar bug fix & test cases"

This reverts commit 640dd55.

* sonar bug fix & test cases

* sonar bug fix & test cases

* sonar bug fix & test cases

* sonar bug fix : avoid Thread.sleep

* sonar bug fix : cleanup Thread.sleep

* sonar bug fix: test commit

* sonar bug fix: test commit

Co-authored-by: Subhrodip Mohanta <[email protected]>
Co-authored-by: atayal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ohbus ohbus ohbus approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@ohbus ohbus

Labels

status: under review

Projects

None yet

Milestone

1.26.0

Development

Successfully merging this pull request may close these issues.

3 participants

@interactwithankush @ohbus @iluwatar