lucene-solr
lucene-solr copied to clipboard
SOLR-14731: Add SingleThreaded Annotation to Class
Description
Makes use of @anshumg and @markrmiller's single threaded annotation so that it is more obvious to maintainers.
Solution
adds it.
Tests
Tests do not change for this one.
Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the
masterbranch. - [ ] I have run
ant precommitand the appropriate test suite. - [x] I have added tests for my changes.
- [x] I have added documentation for the Ref Guide (for Solr changes only).
simple PR here, will the package masters @chatman and @noblepaul please take a look, as well as @anshumg. I'll use this annotation going forward as applicable. Thanks for adding it! ☺️
The annotation usage is correct, however I haven't taken a look at the code itself to validate if it's meant to be single threaded or not. Perhaps other folks who've been in here can chime in on that.
I don't really know what purpose is solved by this annotation. If it is for documentation purpose, this may make sense. But, there are a million other places where the classes are not thread-safe. What's the point of just adding it here
Rome was not built in one day, so too our code documentation Is in not finished in one PR
@anshumg
The annotation usage is correct, however I haven't taken a look at the code itself to validate if it's meant to be single threaded or not. Perhaps other folks who've been in here can chime in on that.
@anshumg are you unable to look at it? It's one file.
I'm not sure what the end result is of adding all these annotations in the short term, but if it is very challenging to get them reviewed for something as simple as this one, do you think I should invest the time? Also, maybe @noblepaul but feels like he is doing a lot in Solr atm..
Looks good. I'll merge this soon. Thanks @MarcusSorealheis.
@chatman Can you re-run this test and commit this one as well?
are we able to first the tests to run again?