Support exceptions.

[Timandes]

Thanks to @ryanuber 's Pull request #28.

We need more tests after merging.

[andreiz]

We should probably bump the version significantly, because exceptions will most likely mean people need to modify their code for it to continue functioning.

[faabiosr]

I agree with you @andreiz!
@Timandes , is very important that you make tests for this PR.

[faabiosr]

@andreiz How we could resolve this issue about version in pecl repository?

[andreiz]

We update the version here, then build a PECL tarball and upload it.

[faabiosr]

Awesome @andreiz !
Please, don't build the package now, wait @Timandes make tests.

[andreiz]

I'm not building anything until this is merged in.

[faabiosr]

👍

[Timandes]

@fabiorphp I'm not good at writing test case(maybe because i'm not good at english). Could you guide me to do this?

[faabiosr]

Of course! I'm here to help you.

Before we start, please look at the tests that are already created. The tests are very easy.

[Timandes]

OK, i'll start with existing case.
I saw a lot of fails on travis-ci.....

[faabiosr]

It's normal. If you prefer run in your machine, please look the dev-tools folder.

I recommend that you install the Zookeeper server for you create integration tests.

[faabiosr]

Let us wait the @andreiz integrating the repository with Travis-CI and Coveralls, it's more easy to approve pull-requests

[Timandes]

@fabiorphp Wait a moment.
I found some compiling errors under PHP with ZTS enabled.

[Timandes]

done : )

[faabiosr]

I will test soon.

[faabiosr]

I think the tests must be more simple.
E.g:

--TEST--
Should throw exception when check if node exists without connect
--SKIPIF--
<?php
if (!extension_loaded('zookeeper')) {
    echo 'Zookeeper extension is not loaded'
};
--FILE--
<?php
$client = new Zookeeper();
try {
    $client->exists('/test1');
} catch(Exception $e) {
    echo $e->getMessage() . "\n" . $e->getCode();
}
--EXPECT--
Zookeeper->connect() was not called
5998

[Timandes]

Does --EXPECT-- part support PHP code?

like:

--EXPECT--
Zookeeper->connect() was not called
<?php echo Zookeeper::CONNECT_NOT_CALLED; ?>

I'm worried about future changing of constant value.

Is this my over-worriness?

[faabiosr]

Please, see these docs bellow and be a monster in making tests!

https://qa.php.net/write-test.php
https://qa.php.net/phpt_details.php

[Timandes]

Monster +_+....

[Timandes]

Done.

[faabiosr]

Well, @Timandes all tests passed!
Now, we wait @andreiz merge this PR.

@andrei look at the Travis-CI integration guide:
http://docs.travis-ci.com/user/getting-started/

[faabiosr]

Anything new?

[andreiz]

Sorry, I'll try to do this today.

On Mon, Jun 15, 2015 at 3:28 AM, Fábio da Silva Ribeiro <
[email protected]> wrote:

Anything new?

—
Reply to this email directly or view it on GitHub
#59 (comment).

[faabiosr]

Awesome @andreiz 👍

[andreiz]

So what should go into .travis.yml file, just this?

language: php
php:

• 5.4
• 5.5
• 5.6

On Mon, Jun 15, 2015 at 10:56 AM, Fábio da Silva Ribeiro <
[email protected]> wrote:

Awesome @andreiz https://github.com/andreiz [image: 👍]

—
Reply to this email directly or view it on GitHub
#59 (comment).

[faabiosr]

The Travis file was created and is on the master branch:
https://github.com/andreiz/php-zookeeper/blob/master/.travis.yml.

You should only enable for this repo.

[andreiz]

I enabled the TravisCI integration and kicked off a build.

On Tue, Jun 16, 2015 at 2:09 PM, Fábio da Silva Ribeiro <
[email protected]> wrote:

The Travis file was created and is on the master branch:
https://github.com/andreiz/php-zookeeper/blob/master/.travis.yml.

You should only enable for this repo.

—
Reply to this email directly or view it on GitHub
#59 (comment).

[faabiosr]

Awesome! 👍

The next step is enable Coveralls for coverage!

[faabiosr]

@Timandes Can you make push --force for run Travis-CI integration?

[Timandes]

@fabiorphp How should i do that?

[faabiosr]

It is simple!
Use commit ammend and after that, push force

[Timandes]

$ git push --force
warning: push.default is unset; its implicit value is changing in
Git 2.0 from 'matching' to 'simple'. To squelch this message
and maintain the current behavior after the default changes, use:

  git config --global push.default matching

To squelch this message and adopt the new behavior now, use:

  git config --global push.default simple

See 'git help config' and search for 'push.default' for further information.
(the 'simple' mode was introduced in Git 1.7.11. Use the similar mode
'current' instead of 'simple' if you sometimes use older versions of Git)

Username for 'https://github.com': Timandes
Password for 'https://[email protected]':
Everything up-to-date

I'm not sure that i did a right thing ...

[faabiosr]

Well, is git push push -f origin master in you repo.

Be careful!

[Timandes]

Is there anything else i can do?

[faabiosr]

Not know. We will wait the @andreiz decide.

[Timandes]

You never sleep? ...
Why do i always see you online whenever i'm online?...

[faabiosr]

Never sleep. Sleep is for the weak.

[Timandes]

洗洗睡吧……

[andreiz]

Where is PHPZK_ADDITIONAL_CONSTANTS ever defined?

[Timandes]

Maybe it's useless.
Should I remove it?

[Timandes]

Log from Travis-CI:

--2015-06-30 01:10:19--  (try:20)  http://apache.fayea.com/zookeeper/zookeeper-3.4.6/zookeeper-3.4.6.tar.gz
Connecting to apache.fayea.com (apache.fayea.com)|119.6.56.18|:80... failed: Connection timed out.
Giving up.

Should I change mirror of Zookeeper package?

[Timandes]

Is there anything more I can do?

[faabiosr]

Something new guys?

[Timandes]

Hope so.

[andreiz]

Is this ready to be merged then?

[Timandes]

👌

[faabiosr]

Yep!

[faabiosr]

Something new guys?

[faabiosr]

Something new guys? Or that PR will stand still as the other PR #28

[faabiosr]

Can I merge this request? I have permission to do that.

[andreiz]

I'll bump the version and make a release.