Skip to content

Conversation

tapan-kumar-thapa
Copy link

Hi Team,

I have added ignoreTube as well as getResponseLinesText method @ incubator/Library/Phalcon/Queue/Beanstalk/Extended.php as per beanstalk protocol defination (https://github.com/kr/beanstalkd/blob/master/doc/protocol.txt) to ignore any watched tube.

It is working fine at my laptop and server. Please review and provide this feature to rest of world.

Thanks in advance for providing such a great framework.

Regards
Tapan Thapa

@xboston
Copy link
Contributor

xboston commented Jan 10, 2014

Good =)
@endeveit , please check this?

@endeveit
Copy link
Contributor

Everything seems well except code style.

  1. The name of method «getResponseLinesText» is not semantic maybe it should be named something like «getWatchingResponse»
  2. There is no phpDoc type named «text» only «string»
  3. In reformat code #79 the code style indentation was changed to tabs

@tapan-kumar-thapa
Copy link
Author

  1. method name getResponseLinesText has been changed to getWatchingResponse to make best practice in place.
  2. Code formatting has been changed from SPACES to TAB.
  3. Correct phpdoc has been set on getWatchingResponse method from text to string.

…sponse to make best practice in place. 2. Code formatting has been changed from SPACES to TAB. 3. Correct phpdoc has been set on getWatchingResponse method from text to string.
@xboston
Copy link
Contributor

xboston commented Jan 11, 2014

@endeveit good?

@endeveit
Copy link
Contributor

Here is the unified diff for additional code style fixes https://gist.github.com/endeveit/8368438

@tapan-kumar-thapa
Copy link
Author

patched applied.

@endeveit
Copy link
Contributor

👍

@tapan-kumar-thapa
Copy link
Author

Also would like to know which text editor you are using with which formatting plugin. I am using sublime text 2.

@endeveit
Copy link
Contributor

I'm using Intellij Idea with PHP plugin.
But source code formatting i do with php-cs-fixer.

@tapan-kumar-thapa
Copy link
Author

Thanks for the info. Let me know if my code looks ok now. If yes than please merge it or confirm if any deviation.

@endeveit
Copy link
Contributor

@xboston now everything is good

xboston added a commit that referenced this pull request Jan 11, 2014
ignoreTube method added @ incubator/Library/Phalcon/Queue/Beanstalk/Exte...
@xboston xboston merged commit f044855 into phalcon:master Jan 11, 2014
@xboston
Copy link
Contributor

xboston commented Jan 11, 2014

Thanks guys!

@tapan-kumar-thapa
Copy link
Author

Your welcome...
I am glad that i have given something back to community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants