Skip to content

Conversation

@marilynel
Copy link
Contributor

No description provided.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 7, 2025

return false;
}
}

Copy link
Owner

Choose a reason for hiding this comment

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

Please add JavaDocs for this method

return true;
}

private boolean checkThis(Object valueThis, Object valueOther) {
Copy link
Owner

Choose a reason for hiding this comment

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

Please add JavaDocs for this method

public void issue743SerializationMapWith1000Objects() {
HashMap<String, Object> map = buildNestedMap(1000);
JSONParserConfiguration parserConfiguration = new JSONParserConfiguration().withMaxNestingDepth(1000);
HashMap<String, Object> map = buildNestedMap(500);
Copy link
Owner

Choose a reason for hiding this comment

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

Please add a comment explaining why the limit was changed. Also, method signature should be updated to '500Objects'.

Not why this testcase started failing in laptop tests.

@stleary
Copy link
Owner

stleary commented Sep 8, 2025

What problem does this code solve?
Address sonarqube issues in JSONObject. More to come.

Does the code still compile with Java6?
Yes

Risks
Low

Changes to the API?
No

Will this require a new release?
No

Should the documentation be updated?
No

Does it break the unit tests?
No. There is one unit testcase that has been breaking in local builds, this was fixed. More investigation is needed, but not for this PR.

Was any code refactored in this commit?
All of the code changes were for refactoring.

Review status
APPROVED

Starting 3-day comment window.

@marilynel Please fix the JavaDoc comments, if you get a chance before merge. Otherwise, they can be included in the next batch of SonarQube fixes. Also, not sure why SonarQube is reporting a new issue for code that hasn't changed in more than a decade. Will address it with the rest of the issues.

@stleary stleary merged commit a3edc1d into stleary:master Sep 11, 2025
8 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