-
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
Small improvements: indentation and syntax #23647
Conversation
Minor improvements and additions in the indentation (1 line instead of 2) and in the syntax of some comments.
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
// we have that problem when we have an empty index is only a segments_1 file then we can't tell if it's a Lucene 4.8 file | ||
// and therefore no checksum. That isn't much of a problem since we simply copy it over anyway but those files come out as | ||
// if we have different files then they must have no checksums; otherwise something went wrong during recovery. | ||
// we have that problem when we have an empty index (is only a segments_1 file) then we can't tell if it's a Lucene 4.8 file |
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.
I think this should be: "we have an empty index is only a segments_1 file so we can't tell"
@@ -705,7 +702,7 @@ public String toString() { | |||
/** | |||
* Represents a snapshot of the current directory build from the latest Lucene commit. | |||
* Only files that are part of the last commit are considered in this datastrucutre. | |||
* For backwards compatibility the snapshot might include legacy checksums that | |||
* For backwards compatibility, the snapshot might include legacy checksums that |
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.
This ,
is optional in English. I don't tend to include it most of the time so I wouldn't change this.
@nik9000 I did the corrections according to your comments. For anything else that may be needed, please let me know. Thank you. |
Thanks for doing those @nikiforosbotis! I've merged and backported to 5.x and 5.3. |
Thank you @nik9000 ! I will try to do more improvements at the elastic search's source code. |
* master: Fix typo in allocation explain API docs Add unit tests for ReverseNestedAggregator (elastic#23651) Revert "Revert "Build: Upgrade min gradle to 3.3 (elastic#23544)"" Revert "Build: Upgrade min gradle to 3.3 (elastic#23544)" Build: Upgrade min gradle to 3.3 (elastic#23544) Fix took assertion in response filter test Search took time should use a relative clock Adds toString() to snapshot operations in progress Docs: fix a typo in transport client's put-mapping.asciidoc (elastic#23607) Use include-tagged macro for high level client docs (elastic#23438) Update fill-column in .dir-locals.el to 100 characters Setup keystore during integration tests (elastic#22966) Fix typo 'Elastisearch' -> 'Elasticsearch' (elastic#23633) Comment and blank line cleanups (elastic#23647) docs: guidelines for students and teachers (elastic#23648) Fix MapperService StackOverflowError (elastic#23605)
Minor improvements and additions in the indentation (1 line instead of 2) and in the syntax of some comments.