Added timeout #2

[acasar]

This fixes BC problem with #17 (original issue: #16) so that it works even if the timeout config is not defined. I successfully tested the implementation on a real project and I also cleaned up the original PR to remove all unnecessary changes.

Can this be merged?

[tshafer]

Im going to review all pr’s and issues tonight for all of collective reps.

— 
Tom Shafer

From: Anže Časar notifications@github.com
Reply: LaravelCollective/remote reply@reply.github.com
Date: November 23, 2015 at 8:36:09 AM
To: LaravelCollective/remote remote@noreply.github.com
Subject:  [remote] Added timeout #2 (#18)

This fixes BC problem with #17 (original issue: #16) so that it works even if the timeout config is not defined. I successfully tested the implementation on a real project and I also cleaned up the original PR to remove all unnecessary changes.

Can this be merged?

You can view, comment on, or merge this pull request online at:

  #18

Commit Summary

Implemented timeout
File Changes

M config/remote.php (1)
M src/Connection.php (7)
M src/RemoteManager.php (4)
M src/SecLibGateway.php (13)
M tests/RemoteSecLibTest.php (2)
Patch Links:

https://github.com/LaravelCollective/remote/pull/18.patch
https://github.com/LaravelCollective/remote/pull/18.diff
—
Reply to this email directly or view it on GitHub.

[ikari7789]

@acasar I'm failing to see how this is any different from my implementation? What did you add?

[ikari7789]

@acasar I saw the difference finally.
I added the same check to mine along with checks for host and username as well. I don't think there is a need for two pull requests.

[acasar]

@ikari7789 I detailed the BC problem in the original issue #16 but I presumed you didn't have time to fix it since there was no reply. So I made the change myself and also cleaned up some unnecessary changes in docblocks along the way.

Before I close this PR, it seems there are still some CS issues in your updated PR? I don't mind either way, as long as this issue is fixed :)

[ikari7789]

Yeah, I'll fix it up tomorrow at work.
Sorry for the lack of reply.

Sent from my iPhone

On Nov 24, 2015, at 19:13, Anže Časar notifications@github.com wrote:

@ikari7789 I detailed the BC problem in the original issue #16 but I presumed you didn't have time to fix it since there was no reply. So I made the change myself and also cleaned up some unnecessary changes in docblocks along the way.

Before I close this PR, it seems there are still some CS issues in your updated PR? I don't mind either way, as long as this issue is fixed :)

—
Reply to this email directly or view it on GitHub.

[ikari7789]

I fixed the StyleCI warnings and removed the /** @var Connection $connection **/ comments.
The other docblock changes are to reflect the new version of PHPseclib and I don't really feel like being bothered to make a new pull request just to update the docblocks (when I'm the one who should have updated them in the first place with my pull request that updated PHPseclib).

[acasar]

@ikari7789 Thanks. Hope this get merged soon :)