Visitar URL original
Implement daemon mode by andig · Pull Request #351 · php-pm/php-pm · GitHub
Skip to content

Implement daemon mode - #351

Open
andig wants to merge 5 commits into
php-pm:masterfrom
andig:daemon
Open

andig wants to merge 5 commits into
php-pm:masterfrom
andig:daemon

Conversation

@andig

@andig andig commented Feb 4, 2018

Copy link
Copy Markdown
Contributor

Some code is borrowed from https://github.com/phlib/console-process (LGPL)- need to double-check if that is ok

@szepeviktor

Copy link
Copy Markdown
Contributor

Any word about it in the README?

@marcj

marcj commented Feb 5, 2018

Copy link
Copy Markdown
Member

We can't use code from a different license if it's just simply copied. If it's inspired or is a construct that is used also everywhere else (like in the documentation, tutorials etc) then we don't need the reference to phlib/console-process as those stuff is not licensable. I personally don't see anything that is defendable although I haven't checked the origin and don't know how hard you copied, if you even did.

@andig

andig commented Feb 5, 2018

Copy link
Copy Markdown
Contributor Author

We can't use code from a different license if it's just simply copied.

Imho they could chose to relicense for us. I've opened an issue at that project. If you have objections please feel free to close but I'm lacking capacity to start over :/

@chrisminett

Copy link
Copy Markdown

Hi,

I'm a maintainer at phlib/console-process

It's fine with us! See comment at phlib/console-process#4

Cheers,
ChrisM

@andig
andig force-pushed the daemon branch 3 times, most recently from 665c8fd to 4f36839 Compare August 24, 2018 17:22
@andig

andig commented Aug 24, 2018 •

Copy link
Copy Markdown
Contributor Author

I've updated and simplified the PR.
I would appreciate a second look as the PHP 7 build fails with an unclear error message. Not sure if this is intermittent or real?

@andig
andig force-pushed the daemon branch 5 times, most recently from 83b15b1 to 30d126a Compare August 27, 2018 16:06
@andig andig added this to the 2.0 milestone Aug 27, 2018
@marcj

marcj commented Oct 22, 2018

Copy link
Copy Markdown
Member

What is the main processes dies? Will it automatically restart?

@andig

andig commented Oct 22, 2018

Copy link
Copy Markdown
Contributor Author

No, thats nothing I‘ve considered. Should be done via systemctrl daemon setting?

@mowangjuanzi

Copy link
Copy Markdown

When will it be merged to master? I need it

@acasademont

Copy link
Copy Markdown
Contributor

Hi @andig should we move forward with this one?

if ($output->isVerbose()) {
$output->writeln('Daemonized');
}
return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this project supports Symfony 5, the return value of a commands execute method must be an integer. While it is not part of the methods signature in the abstract Command class, it is still validated and an exception will be thrown for any non-integer return value. So I suggest, you change this to return 0; along with the other return statement below.

@andig

andig commented Aug 7, 2020

Copy link
Copy Markdown
Contributor Author

Hi @andig should we move forward with this one?

Up to you. We‘ve had this around for so long I wouldn‘t mind closing although there seems to be some positive comments.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants