Skip to content
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.

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

Remove unnecessary index writer getting opened #825

Conversation

laimis
Copy link
Contributor

@laimis laimis commented Apr 12, 2023

Continuation of fixes with virtual calls being made from constructors. The issue originally reported by SonarCloud scans: https://sonarcloud.io/project/issues?resolved=false&rules=csharpsquid%3AS1699&id=apache_lucenenet and referenced in this issue: #670

This one focuses on AnalyzingInfixSuggester. After going back to Lucene repository and looking at the history of the file, I found this:

https://issues.apache.org/jira/browse/LUCENE-7670

and apache/lucene@c1fe88b

The opening of a writer appears to be a bug that was fixed, and the line was removed. Removing the line eliminates the sonar cloud issue as well.

@laimis laimis requested a review from NightOwl888 April 13, 2023 04:42
@laimis
Copy link
Contributor Author

laimis commented Apr 13, 2023

@NightOwl888 this is ready for the review. I was going to port the test changes, but that test is not there yet in our version and the AnalyzingInfixSuggester will undergo changes from the version we have to the version that's references in 2017 JIRA ticket. I am leaving the test out. I feel comfortable with our current test coverage for this class that we can proceed as is.

@laimis laimis merged commit 8a7c112 into apache:master Apr 13, 2023
@laimis laimis deleted the fix/virtual-calls-from-constructors-analyzinginfixsuggester branch April 13, 2023 18:30
NightOwl888 added a commit that referenced this pull request Apr 23, 2023
NightOwl888 added a commit that referenced this pull request Apr 23, 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.

2 participants