SOLR-18352: Improve DX by more accurately communicating deprecated code status - #4750
Conversation
dsmiley
left a comment
dsmiley
left a comment
There was a problem hiding this comment.
Please reduce the scope to the PR title.
There was a problem hiding this comment.
Arguably this entire package can be seen as having a consistent status: The code is associated with "Trie" numerics. Anticipated to be fully resolved (replacements) for Solr 11. @HoustonPutman is leading the way here with progress.
Maybe the BBox stuff could separately be decoupled from that and be ported to Points + NumericDocValues but someone has not undertaken that.
There was a problem hiding this comment.
I think that if you created a nice JIRA for that, then it COULD be.... I don't know enough about BBox to say.
There was a problem hiding this comment.
These deprecated methods were deprecated on 10.0; look at branch_10_0 to see for yourself. Thus we may remove them anytime in 10.x. 9.x had no alternative so devs that worked on this didn't deprecate there.
There was a problem hiding this comment.
i thought if we deprecated on 10, then we remove on 11? I will go back and check the deprecation tags for the 10 versus 10.1...
There was a problem hiding this comment.
Arguably we could have just removed/changed those methods outright in a major version. It's a major version, after all!
Co-authored-by: David Smiley <dsmiley@apache.org>
|
I will aim to get this merged tomorrow against |
|
@dsmiley re-requesting review, I have borught this up to date and tweaked some of the language... |
dsmiley
left a comment
dsmiley
left a comment
There was a problem hiding this comment.
+1 from me, just looking at your last few commits separately. For some reason, GitHub is not able to show the diffs from the last merge base; instead it's showing the changes from the merge as if you had done them -- which is impossible to effectively review (80% other stuff, 20% your changes) on an already big PR.
Note: Since you are doing this against main, you are running into things getting removed and so you're removing the comments. Had this been on 9x, we'd been able to keep those comments. The forward port process would naturally remove what doesn't exist on higher versions.
| public void setUp() { | ||
| MockitoAnnotations.initMocks(this); | ||
|
|
||
| // handleItem() probes the cluster through the state provider, so the mock must supply one |
There was a problem hiding this comment.
This is the minimum to get the unit tests to pass... However, I am opening on up a new seperate PR that does a better job of handling Mocks for the crossdc manager tests.
There was a problem hiding this comment.
You caught the reason why CI was failing & fixed; thanks. I hate mocks -- returning null when the real thing would never do that
|
tests pass locally now. |
2 participants
https://issues.apache.org/jira/browse/SOLR-18352
Description
Expand our
@deprecatedtag details with more info.Solution
Mostly mechnaical work with claude, use git history to figure out when a deprecation was added, and then use that for the
sincetag. Look for git histories to help understand what replaced deprecated code if that is available.Tests
existing