Skip to content

Conversation

@olcbean
Copy link
Contributor

@olcbean olcbean commented Mar 14, 2018

As discussed in #28943 (comment) and #28866 (comment), the deprecated url parameters should be removed.

@elasticmachine
Copy link
Collaborator

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?

1 similar comment
@elasticmachine
Copy link
Collaborator

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?

@nik9000 nik9000 added the :Core/Infra/REST API REST infrastructure and utilities label Mar 14, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@nik9000
Copy link
Member

nik9000 commented Mar 14, 2018

@olcbean, looks good to me!

@elasticmachine, test this please.

@nik9000 nik9000 merged commit 3d81497 into elastic:master Mar 14, 2018
@nik9000
Copy link
Member

nik9000 commented Mar 14, 2018

Thanks @olcbean! I've merged this to just master.

public static final ParseField FIELDDATA = new ParseField("fielddata", "field_data");
public static final ParseField QUERY = new ParseField("query");
public static final ParseField REQUEST = new ParseField("request");
public static final ParseField FIELDDATA = new ParseField("fielddata");
Copy link
Member

Choose a reason for hiding this comment

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

at this point using ParseField s for parameters can be replaced with just strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch!
Do you wanna take it over or should I?

Copy link
Member

Choose a reason for hiding this comment

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

all yours if you have time for it ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants