Skip to content

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Sep 19, 2025

Reference URL

Instead of the usual Jira issue, here is a link to the list of quality problems found on SonarQube:
https://sonarcloud.io/project/issues?fileUuids=AXnpASDXDDFOvAKXAP6t&languages=js&issueStatuses=OPEN%2CCONFIRMED&id=org.xwiki.platform%3Axwiki-platform

Changes

Description

  • Removed the 22 quality "blocker" issues by declaring the javascript variables properly.
  • Removed easy to remove high->low quality issues. 8 issues left (XWiki.widgets.Notification unused instantiations and Cognitive Complexity)

Clarifications

  • It's far from being perfect but it reduces the number of quality issues found from 94 to 8, without introducing any new one.

Screenshots & Video

Test issue only.
image

Executed Tests

SonarQube validation (see screenshot above)
Built the change successfully with mvn clean install -f xwiki-platform-core/xwiki-platform-web/xwiki-platform-web-war -Pquality
Then passed the test suite successfully: mvn clean install -f xwiki-platform-core/xwiki-platform-panels/xwiki-platform-panels-test/xwiki-platform-panels-test-docker/

Tested manually on a local instance to make sure that changes did not influence functionality. At some point I tried changing the scope of variables and this broke toolTip.js (which relies on values of globally defined variables...) but the current solution works well.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • None

* Removed the 22 quality "blocker" issues by declaring the javascript variables properly.
* Removed easy to remove high->low quality issues. 8 issues left (XWiki.widgets.Notification unused instantiations and Cognitive Complexity)
@Sereza7 Sereza7 requested a review from mflorea October 8, 2025 15:18
Copy link
Member

@mflorea mflorea left a comment

Choose a reason for hiding this comment

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

Looks good. I left a few suggestions, but can be merged as is.

Sereza7 and others added 6 commits October 10, 2025 17:08
* Updated `let` assignments to be `const`

Co-authored-by: Marius Dumitru Florea <[email protected]>
* Updated `let` assignments to be `const`

Co-authored-by: Marius Dumitru Florea <[email protected]>
* Updated `let` assignments to be `const`

Co-authored-by: Marius Dumitru Florea <[email protected]>
* Updated `let` assignments to be `const`

Co-authored-by: Marius Dumitru Florea <[email protected]>
* Updated syntax

Co-authored-by: Marius Dumitru Florea <[email protected]>
* Removed unused variable assignment
@Sereza7
Copy link
Contributor Author

Sereza7 commented Oct 10, 2025

Thanks for the help @mflorea !
I reran the sonarQube tests, found a small problem added with your proposed changes and updated the PR.
SonarQube doesn't find any new issue with the file.
I tested it on my local instance and it still works as expected.

This PR is ready for further review or a merge :)

@Sereza7 Sereza7 requested a review from mflorea October 10, 2025 15:16
@mflorea mflorea merged commit e6c3046 into xwiki:master Oct 22, 2025
2 checks passed
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