[DX] Version method #1152

Merged
merged 3 commits into from Aug 8, 2016

Conversation

Projects
None yet
2 participants
@oleg-andreyev
Contributor

oleg-andreyev commented Jul 27, 2016

adding getVersion method to Client.

Case: For large platforms it's critical to upgrade incrementally/step-by-step, for this purpose we need to know current version on ES

+ public function getVersion()
+ {
+ if ($this->_version) {
+ return $this->_version;

This comment has been minimized.

@ruflin

ruflin Jul 31, 2016

Owner

Is there a reason you "cache" the version? Do you expect that this call is happening more then once during the lifetime of a Client?

@ruflin

ruflin Jul 31, 2016

Owner

Is there a reason you "cache" the version? Do you expect that this call is happening more then once during the lifetime of a Client?

This comment has been minimized.

@oleg-andreyev

oleg-andreyev Aug 3, 2016

Contributor

It's possible. Imagine two repositories: A, B both are using ES, both should be B/C with 1.7 and 2.0.

@oleg-andreyev

oleg-andreyev Aug 3, 2016

Contributor

It's possible. Imagine two repositories: A, B both are using ES, both should be B/C with 1.7 and 2.0.

test/lib/Elastica/Test/ClientTest.php
+ {
+ $client = $this->_getClient();
+ $this->assertNotEmpty($version = $client->getVersion());
+ $this->assertSame($version, $client->getVersion());

This comment has been minimized.

@ruflin

ruflin Jul 31, 2016

Owner

Could we use here some like version compare to compare it >= 2.0.0? http://php.net/manual/en/function.version-compare.php

@ruflin

ruflin Jul 31, 2016

Owner

Could we use here some like version compare to compare it >= 2.0.0? http://php.net/manual/en/function.version-compare.php

This comment has been minimized.

@oleg-andreyev

oleg-andreyev Aug 3, 2016

Contributor

I don't think we should hardcode version of ES that is used for tests.
I'll take a look, if it's possible to expose ES_VERSION and use it here.

@oleg-andreyev

oleg-andreyev Aug 3, 2016

Contributor

I don't think we should hardcode version of ES that is used for tests.
I'll take a look, if it's possible to expose ES_VERSION and use it here.

This comment has been minimized.

@ruflin

ruflin Aug 3, 2016

Owner

I think it is fine to "hardcode" the minimal compatible version. >=2.0.0 will work for all future versions. I agree if we "hardcode" it, it should be in a variable like MINIMAL_COMPATIBLE_VERSION for reusability and this makes it easier to update it. If you find a good way to expose ES_VERSION, also nice.

@ruflin

ruflin Aug 3, 2016

Owner

I think it is fine to "hardcode" the minimal compatible version. >=2.0.0 will work for all future versions. I agree if we "hardcode" it, it should be in a variable like MINIMAL_COMPATIBLE_VERSION for reusability and this makes it easier to update it. If you find a good way to expose ES_VERSION, also nice.

This comment has been minimized.

@oleg-andreyev

oleg-andreyev Aug 3, 2016

Contributor

I've added new ENV to Dockerfile and then checking it in test, see 0abec0e

@oleg-andreyev

oleg-andreyev Aug 3, 2016

Contributor

I've added new ENV to Dockerfile and then checking it in test, see 0abec0e

@ruflin

This comment has been minimized.

Show comment
Hide comment
@ruflin

ruflin Jul 31, 2016

Owner

Thanks, definitively a useful addition. See my comments above.

Owner

ruflin commented Jul 31, 2016

Thanks, definitively a useful addition. See my comments above.

@ruflin ruflin merged commit 9ef242b into ruflin:master Aug 8, 2016

2 checks passed

codecov/project 85.21% (+0.01%) compared to a714773
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ruflin

This comment has been minimized.

Show comment
Hide comment
@ruflin

ruflin Aug 8, 2016

Owner

@oleg-andreyev Thanks. Merged. This will be part of the next release.

Owner

ruflin commented Aug 8, 2016

@oleg-andreyev Thanks. Merged. This will be part of the next release.

@oleg-andreyev oleg-andreyev deleted the oleg-andreyev:version-method branch Jun 18, 2018

@oleg-andreyev oleg-andreyev restored the oleg-andreyev:version-method branch Jun 27, 2018

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