Convert more classes to record classes

apache / lucene

Apache Lucene open-source search software

https://lucene.apache.org/

Apache License 2.0

2.45k stars 975 forks source link

Convert more classes to record classes #13328

Open shubhamvishu opened 2 months ago

shubhamvishu commented 2 months ago

Description

This PR addresses #13207 to convert more classes on main branch to record classes on main (Lucene 10 only).
It moves a lot of data classes(120 to be precise) to use record classes that were flagged suitable in my IDE and seemed good candidate in general.
I left a couple of them like TotalHits and SynonymMap as the PR is already very big and including those were leading to a lot more changes. Maybe we could do those as a separate PR.

Raising this PR since all the tests are passing(./gradlew test) but renderJavadoc task is complaining about missing java docs on some record classes(converted in this PR) which I see has the javadocs already eg: TermStats, ReaderSlice. I'm not sure why its flagging those incorrectly or if maybe I'm missing something.

uschindler commented 1 month ago

Hi, you can merge main into your branch and hopefully checks pass. For some packages like the "codecs" one, the new code requires you to also document all record components with @param.

shubhamvishu commented 1 month ago

I addressed your comments in the new revision and all the checks are passing now after your fix. Thanks!

uschindler commented 1 month ago

Thanks, looks fine. I have no time to do another closer review, please give me some time to proceed.

github-actions[bot] commented 1 month ago

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

uschindler commented 1 month ago

Hi @shubhamvishu, can you check my comments? The changes here look fine, please fix the remaining issues. Uwe

shubhamvishu commented 1 month ago

Hi @uschindler , sorry I was on vacation until last week, so this PR stalled. I'll take a look at the comments today or tomorrow.

Update : Got stuck with some other work. Will take a look at this sometime this week.

shubhamvishu commented 3 weeks ago

Hi @uschindler , thanks for the review. I took another pass at the changes and pushed some commits addressing your comments. Please have a look when you get a chance. Thanks!

github-actions[bot] commented 3 days ago

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!