Skip to content

Conversation

@rjernst
Copy link
Member

@rjernst rjernst commented Oct 23, 2018

This commit removes the last non context based script class.

This commite removes the last non context based script class.
@rjernst rjernst added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v7.0.0 >refactoring labels Oct 23, 2018
@rjernst rjernst requested a review from jdconrad October 23, 2018 02:36
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

This was accidentally left over when converting to FieldScript.

closes elastic#34683
Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

@rjernst This is awesome! Left one comment and adding one request - the change of contexts here puts the Painless docs out of date. Doesn't have to be this PR, but would you please update the context docs to reflect the changes at some point before 7.0?

Compiler compiler = contextsToCompilers.get(context);

if (context.instanceClazz.equals(SearchScript.class)) {
Constructor<?> constructor = compile(compiler, scriptName, scriptSource, params);
Copy link
Contributor

Choose a reason for hiding this comment

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

There are two versions of the compile method. Now that the two special cases are gone for search/executable script contexts, would you please check to see if this version can be removed completely? It's possible some tests still use it, though.

@rjernst rjernst merged commit 687dc1e into elastic:master Oct 24, 2018
@rjernst rjernst deleted the remove_search_script branch October 24, 2018 22:03
kcm pushed a commit that referenced this pull request Oct 30, 2018
This commit removes the last non context based script class.
jdconrad added a commit that referenced this pull request Nov 7, 2018
With the removal of SearchScript (#34730), an extraneous compile method 
was left behind in PainlessScriptEngine. This change removes the method and 
updates the tests that depend on it to use the main compile method which 
gives better test coverage.
pgomulka pushed a commit to pgomulka/elasticsearch that referenced this pull request Nov 13, 2018
With the removal of SearchScript (elastic#34730), an extraneous compile method 
was left behind in PainlessScriptEngine. This change removes the method and 
updates the tests that depend on it to use the main compile method which 
gives better test coverage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >refactoring v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants