-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Fix erroneous docstrings for abstract bulk by scroll request #37517
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @torgeir, thanks for suggesting the fix to the javadocs. The change looks good to me, but would you go a step further and change the docs to a sentence rather than a question? Something like "Returns "true" of version conflicts cause aborts. This is the default." and something similar (without the "returns) for the two setters?) That would be awesome.
If you don't find the time for an update I can also merge this as-is.
Pinging @elastic/es-distributed |
Sure, I'll reformulate them to sentences without question marks! If you really like to include information about the defaults I'd be happy to do so, but personally I belive that this is a recipe for out of date docs, like the need for this PR shows 😊 |
@cbuescher Had another go, let me know what you think! |
Fair point. Thanks for the change, looks good to me. I will merge this to the other branches that are currently actively developed. |
…-response-header-performance * elastic/master: Remove Redundant RestoreRequest Class (elastic#37535) Create specific exception for when snapshots are in progress (elastic#37550) Mute UnicastZenPingTests#testSimplePings [DOCS] Adds size limitation to the get datafeeds APIs (elastic#37578) Fix assertion at end of forceRefreshes (elastic#37559) Propagate Errors in executors to uncaught exception handler (elastic#36137) [DOCS] Adds limitation to the get jobs API (elastic#37549) Add set_priority action to ILM (elastic#37397) Make recovery source send operations non-blocking (elastic#37503) Allow field types to optimize phrase prefix queries (elastic#37436) Added fatal_exception field for ccr stats in monitoring mapping. (elastic#37563) Fix testRelocateWhileContinuouslyIndexingAndWaitingForRefresh (elastic#37560) Moved ccr integration to the package with other ccr integration tests. Mute TransportClientNodesServiceTests#testListenerFailures Decreased time out in test Fix erroneous docstrings for abstract bulk by scroll request (elastic#37517) SQL: Rename SQL type DATE to DATETIME (elastic#37395) Remove the AbstracLifecycleComponent constructor with Settings (elastic#37523)
Fix erroneous docstrings indicating that
AbstractBulkByScrollRequest::abortOnVersionConflict
defaults tofalse
, when it indeed actually defaults totrue
,