Skip to content
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

Cleanup scan config options. Mark it experimental. #4564

Merged
merged 1 commit into from
Jul 12, 2017

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Jun 27, 2017

Follow up PR from #4374.

Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed a typo that needed fixing, so I went ahead and suggested a few minor edits, too.


If you specify a value other than none for this setting you can determine whether to use ascending or descending order using `scan.order`.
Specifies if files should be harvested in order and how to determine the order. Possible values are `modtime`, `filename`. Leaving the option empty means its disabled. To sort by file modification time use modtime otherwise use filename.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting with the 2nd sentence, change to say:

Possible values are modtime and filename. To sort by file modification time, use modtime, otherwise use filename. Leave this option empty to disable it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed


The default setting is none.
If you specify a value for this setting you can determine whether to use ascending or descending order using `scan.order`.
Copy link
Contributor

@dedemorton dedemorton Jun 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to say: If you specify a value for this setting, you can use scan.order to configure whether files are scanned in ascending or descending order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

Specifies ascending or descending order if `scan.sort` is set to a value other than none. Possible values are asc or desc.
[]experimental

Specifies ascending or descending order if `scan.sort` is set to a value other than none. Possible values are `asc` or `desc`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to say: Specifies whether to use ascending or descending order when scan.sort is set to a value other than none.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

Copy link
Contributor

@codeperfector codeperfector left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ruflin ruflin force-pushed the followup-scan-sort branch 2 times, most recently from ccc7aca to 5ee8cd4 Compare June 28, 2017 07:16

If you specify a value other than none for this setting you can determine whether to use ascending or descending order using `scan.order`.
If you specify a value other than none for this setting you can determine whether to use ascending or descending order using `scan.order`. Possible values are `modtime` and `filename`. To sort by file modification time, use `modtime`, otherwise use `filename`. Leave this option empty to disable it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"other than none" is confusing here, because it used to be the string "none" and now it's the empty string, right? I suggest "other than the empty string".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, change it.

@ruflin ruflin force-pushed the followup-scan-sort branch from 5ee8cd4 to a8f1de4 Compare June 28, 2017 12:23
@andrewkroh andrewkroh self-requested a review July 11, 2017 19:38
@andrewkroh andrewkroh merged commit 8a122a4 into elastic:master Jul 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants