Skip to content
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

Watch command #472

Closed
wants to merge 4 commits into from
Closed

Watch command #472

wants to merge 4 commits into from

Conversation

@ovr
Copy link
Contributor

@ovr ovr commented Aug 4, 2014

Use

zephir watch

Best RFC :))

ovr added 4 commits Aug 4, 2014
- inotifywait add -r flag

- php-fpm restart
@ovr ovr closed this Aug 4, 2014
@ovr ovr reopened this Aug 6, 2014
@@ -18,8 +18,9 @@
"issues": "https://github.com/phalcon/zephir/issues?state=open"
},
"require": {
"php": "~5.3",
"ext-json": "*"
"php": "~5.4",

This comment has been minimized.

@nkt

nkt Aug 6, 2014
Contributor

OMG. Why we should drop 5.3 support?

This comment has been minimized.

@ovr

ovr Aug 6, 2014
Author Contributor

"ext-json": "*"
"php": "~5.4",
"ext-json": "*",
"react/react": "~0.4"

This comment has been minimized.

@nkt

nkt Aug 6, 2014
Contributor

👎
Maybe you can split this code in another project?

This comment has been minimized.

@nkt

nkt Aug 6, 2014
Contributor

You can replace it by react/event-loop

$this->_logger->success(sprintf('Build Success after %.4F sec.', microtime(true)-$start));

if ($restartFPM) {
exec('sudo service php5-fpm restart');

This comment has been minimized.

@nkt

nkt Aug 6, 2014
Contributor

This is very bad hardcode.
We can add before-build, after-build sections, and add this code there. For example there is no service in OS X

This comment has been minimized.

@ovr

ovr Aug 6, 2014
Author Contributor

Will move this hardcode to config.json

@nkt
Copy link
Contributor

@nkt nkt commented Aug 6, 2014

Maybe you should use http://php.net/manual/en/book.inotify.php, and just suggest it in composer.json?

@ovr
Copy link
Contributor Author

@ovr ovr commented Aug 6, 2014

Maybe you should use http://php.net/manual/en/book.inotify.php, and just suggest it in composer.json?

It is not a popular lib, I hate this functional code without async

@nkt
Copy link
Contributor

@nkt nkt commented Aug 6, 2014

@ovr this is not functional code, this is procedure code.
What you expect from asynchrony? For now you doesn't stop build if files changed, so you doesn't use this functionality.

@ovr
Copy link
Contributor Author

@ovr ovr commented Aug 6, 2014

@nkt
asynchronous*

Stop skyped with me and then say:

For now you doesn't stop build if files changed

1 You lib is not supported yet
2 Last update 2012
3 Hosted on pecl not github
4 procedure style
5 no community

@nkt
Copy link
Contributor

@nkt nkt commented Aug 6, 2014

@ovr so what? Who said you that this lib doesn't support?
react is just wrapper for http://php.net/manual/ru/ref.libevent.php. You also use inotifywait, that is not very popular tool. For example there is no inotifywait on OS X.

$ brew search inotifywait
No formula found for "inotifywait".
Searching pull requests...

You wrote command just for you, working just with your environment. This is not ok.

@ovr
Copy link
Contributor Author

@ovr ovr commented Aug 6, 2014

See last commit 2012 year

React can work without any lib
It is not a wrapper
First read something about it and then write

Brew is not official package manager

Inotify is linux kernel component

Maybe I need write command for you?)

I cant support all os because I dont have macos and dont run it under
Windows
I will change hard code for ionotify
06 Авг 2014 г. 22:02 пользователь "Nikita Gusakov" <
notifications@github.com> написал:

@ovr https://github.com/ovr so what? Who said you that this lib doesn't
support?
react is just wrapper for http://php.net/manual/ru/ref.libevent.php. You
also use inotifywait, that is not very popular tool. For example there is
no inotifywait on OS X.

$ brew search inotifywait
No formula found for "inotifywait".
Searching pull requests...

You wrote command just for you, working just with your environment. This
is not ok.


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

@nkt
Copy link
Contributor

@nkt nkt commented Aug 6, 2014

@ovr of course it works. But not asynchronously https://github.com/reactphp/event-loop/blob/master/src/StreamSelectLoop.php#L250
Brew is most popular package manager. Macports also not supports this package.
So, I am 👎

@ovr
Copy link
Contributor Author

@ovr ovr commented Aug 6, 2014

@nkt
https://github.com/reactphp/react/blob/split/composer.json#L26
It works with 3 exstensions But you suggest only 1

As I can see you only writting messages and contribute issues
It's amazing when you send 👎 on my pull requests
But who will write code and added new futures?
I think it would be nice when only @phalcon contributes Zephir 👍

@ovr ovr closed this Aug 6, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.