Skip to content

Conversation

@Mpdreamz
Copy link
Member

@Mpdreamz Mpdreamz commented Oct 5, 2018

Pending #3448

@Mpdreamz Mpdreamz force-pushed the feature/rollup-search branch from f8906fa to 1c27616 Compare October 5, 2018 09:18
Copy link
Contributor

@russcam russcam left a comment

Choose a reason for hiding this comment

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

I left some general comments. Looks like URL tests are missing

// "xpack.rollup.start_job.json",
// "xpack.rollup.stop_job.json",
"xpack.rollup.rollup_search.json",
//"xpack.rollup.rollup_search.json",
Copy link
Contributor

Choose a reason for hiding this comment

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

can delete 😄

public IndexingJobState JobState { get; internal set; }

[JsonProperty("current_position")]
public IDictionary<string, object> CurrentPosition { get; internal set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be IReadOnlyDictionary<string, object>, and possibly initialized?

{
public partial interface IElasticClient
{
/// <inheritdoc/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should have a <summary>...</summary> and all other overloads inheritdoc from it


var si = Sanitize(RandomInitializer);
dict.Add(Integration.ClientMethod.Initializer, request(si, client, initializerBody(si)));
var si = this.Sanitize(RandomInitializer);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we ought to pass across the codebase for 7.x and standardize the conventions 😄

AggregationDictionary IRollupSearchRequest.Aggregations { get; set; }
int? IRollupSearchRequest.Size { get; set; }

/// <summary>When doing rollup searches against rolled up and live indices size needs to be set to 0 explicitly </summary>
Copy link
Contributor

Choose a reason for hiding this comment

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

Move the XML comments to the interface and <inheritdoc />

@russcam russcam merged commit a452215 into feature/rollup-apis Oct 18, 2018
@russcam russcam deleted the feature/rollup-search branch October 18, 2018 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants