Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Irina/sonarjava 4396#4317

Merged
irina-batinic-sonarsource merged 23 commits into
branch-7.16SonarSource/sonar-java:branch-7.16from
irina/SONARJAVA-4396SonarSource/sonar-java:irina/SONARJAVA-4396Copy head branch name to clipboard
Mar 1, 2023
Merged

Irina/sonarjava 4396#4317
irina-batinic-sonarsource merged 23 commits into
branch-7.16SonarSource/sonar-java:branch-7.16from
irina/SONARJAVA-4396SonarSource/sonar-java:irina/SONARJAVA-4396Copy head branch name to clipboard

Conversation

@irina-batinic-sonarsource

Copy link
Copy Markdown
Contributor

No description provided.

@irina-batinic-sonarsource irina-batinic-sonarsource changed the base branch from master to branch-7.16 February 22, 2023 09:30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the great work, I think there is some fine-tuning needed so that we align with the other analyzers but we are almost there

Comment thread java-checks/src/test/java/org/sonar/java/checks/MissingPackageInfoCheckTest.java Outdated
Comment thread java-frontend/src/main/java/org/sonar/java/SonarComponents.java Outdated
Comment on lines +491 to +496
if (!inputFile.status().equals(InputFile.Status.SAME)) {
LOG.trace("File status is: " + inputFile.status() + ". File can be skipped.");
contentHashCache.writeToCache(inputFile);
return false;
}
return canSkipInContext && inputFile.status() == InputFile.Status.SAME;
return contentHashCache.hasSameHashCached(inputFile);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like we are taking a shortcut by checking the status before checking the cache. My understanding is that we should check the cache, and if the cache is disabled, check the status. WDYT?

Comment thread java-frontend/src/main/java/org/sonar/java/caching/ContentHashCache.java Outdated
Comment thread java-frontend/src/main/java/org/sonar/java/caching/ContentHashCache.java Outdated
Comment thread java-frontend/src/main/java/org/sonar/java/caching/ContentHashCache.java Outdated
Comment thread java-frontend/src/main/java/org/sonar/java/caching/ContentHashCache.java Outdated
Comment thread java-frontend/src/main/java/org/sonar/java/caching/ContentHashCache.java Outdated
Comment thread java-frontend/src/test/java/org/sonar/java/SonarComponentsTest.java

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice job on the fixes, I think that should be the last round of ping-pong. Feel free to test the logs in the ContehHashCacheTest tests if you feel it makes sense

Comment thread java-frontend/src/main/java/org/sonar/java/caching/ContentHashCache.java Outdated
Comment thread java-frontend/src/test/java/org/sonar/java/caching/ContentHashCacheTest.java Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍🏿

Comment thread java-frontend/src/main/java/org/sonar/java/caching/ContentHashCache.java Outdated
@sonarsource-next

Copy link
Copy Markdown

@irina-batinic-sonarsource irina-batinic-sonarsource merged commit e985f2c into branch-7.16 Mar 1, 2023
@irina-batinic-sonarsource irina-batinic-sonarsource deleted the irina/SONARJAVA-4396 branch March 1, 2023 17:06
irina-batinic-sonarsource added a commit that referenced this pull request Mar 6, 2023
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.