-
Notifications
You must be signed in to change notification settings - Fork 736
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
Fix script file issues and move script classes into namespace #1028
Conversation
@mortenhauberg Can you update the CHANGELOG.md? |
* @author Nicolas Assing <[email protected]> | ||
* | ||
* @link https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-scripting.html | ||
* Kept for BC reasons. |
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.
Can we add a @deprecated message here? Check some other files which are deprecated for some examples.
LGTM. It would be nice to have at least a deprecated message in the docs in all classes which are deprecated. Here is an example: https://github.com/ruflin/Elastica/blob/f124e598228a6b8a6c249fde6afc07ea7eb1a771/lib/Elastica/Query/Bool.php |
👍 |
Fix script file issues and move script classes into namespace
@mortenhauberg Merged. Thanks. Perhaps in the future we can also trigger an error so it shows up in the logs: Elastica/lib/Elastica/Query/Bool.php Line 4 in f124e59
|
Thank you. I'll create a PR for triggering errors next week :) |
👍 |
Elastica\Script
andElastica\ScriptFile
extendsElastica\AbstractScript
.There are
instanceof
checks and type-hints forElastica\Script
in different classes.I've changed the checks and type-hints to
Elastica\AbstraceScript
, and madeElastica\ScriptFile
extendElastica\Script
.@ruflin suggested to move the scripting classes into a namespace of its own.
We decided to keep the old classes, and have them extend the new classes in
Elastica\Script
for BC reasons.Elastica\AbstractScript
Elastica\Script\AbstractScript
Elastica\Script
Elastica\Script\Script
Elastica\ScriptFile
Elastica\Script\ScriptFile
Elastica\ScriptFields
Elastica\Script\ScriptFields
Please let me know if anything needs changes :)