Skip to content

Conversation

@mbien
Copy link
Member

@mbien mbien commented Jun 16, 2025

  • all lucene deprecations fixed
  • javac warnings fixed
  • JDK 17 language level and other cleanup

Lucene 4.0 breaks API which makes an upgrade a bit tricky since not much builds after that. This moves some of the work to 3.6.2 while tests are still working as attempt to make #8384 easier.

@mbien mbien added Code cleanup Label for cleanup done on the Netbeans IDE ci:all-tests [ci] enable all tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Jun 16, 2025
@mbien mbien added this to the NB27 milestone Jun 16, 2025
@mbien mbien marked this pull request as ready for review June 16, 2025 04:42
@mbien mbien added the Editor label Jun 16, 2025
@mbien mbien requested a review from lkishalmi June 18, 2025 00:19
@mbien mbien force-pushed the lucene-parsing-cleanup branch from da56a2c to 2eeaf73 Compare June 21, 2025 05:02
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

Overall this looks sane to me. I don't agree with everything (for example String#format is in my eyes much more readable than its hot new cousing String#formatted), but I think this is still an improvement.

However at at least one location (see inline comment), I think the API switch changed semantics of the code.

@mbien mbien force-pushed the lucene-parsing-cleanup branch from 2eeaf73 to 52955b6 Compare June 22, 2025 12:10
Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

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

Seems to be fine.

@mbien
Copy link
Member Author

mbien commented Jun 23, 2025

thanks for reviews, I want to simplify the reopen() path a bit since looking at it again today it is more complicated than it needs to be (it can be a null check essentially). Will merge shortly afterwards since the change will be small.

 - all lucene deprecations fixed
 - javac warnings fixed
 - JDK 17 language level and other cleanup
@mbien mbien force-pushed the lucene-parsing-cleanup branch 2 times, most recently from 11de9da to 0e567c7 Compare June 23, 2025 06:48
Comment on lines -986 to 963
if (reader != null) {
final IndexReader newReader = IndexReader.openIfChanged(reader);
IndexReader newReader = IndexReader.openIfChanged(reader);
if (newReader != null) {
reader.close();
reader = newReader;
Copy link
Member Author

Choose a reason for hiding this comment

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

both impls are now the same, this is LuceneIndex

Comment on lines -307 to 296
if (cachedReader != null) {
final IndexReader newReader = cachedReader.reopen();
if (newReader != cachedReader) {
IndexReader newReader = IndexReader.openIfChanged(cachedReader);
if (newReader != null) {
cachedReader.close();
cachedReader = newReader;
Copy link
Member Author

Choose a reason for hiding this comment

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

this is for MemoryIndex

@mbien mbien removed the ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) label Jun 23, 2025
@mbien mbien merged commit 9c9d03e into apache:master Jun 23, 2025
159 of 164 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:all-tests [ci] enable all tests Code cleanup Label for cleanup done on the Netbeans IDE Editor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants