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

Third party lib copyright #848

Closed
Tjatse opened this issue Nov 27, 2014 · 6 comments
Closed

Third party lib copyright #848

Tjatse opened this issue Nov 27, 2014 · 6 comments

Comments

@Tjatse
Copy link
Collaborator

Tjatse commented Nov 27, 2014

The tree-kill lib, it should be wrote by @pkrumins https://github.com/pkrumins/node-tree-kill (2013), but you're using a copy - which with some poor changes (not event change the variable and function names) and appropriate to himself (the copyright header only has himself but no original author), I think we should show our respect to pkrumins, update his codes to match DARWIN system and retain his copyright.

@soyuka
Copy link
Collaborator

soyuka commented Nov 27, 2014

https://github.com/Unitech/PM2/blob/569e938900e3edd4f8a50aa0a8592629883fd7e7/lib/TreeKill.js#L11 are you sure ?

So it's @fengmk2 that should change it's License and mention @pkrumins ?

@Tjatse
Copy link
Collaborator Author

Tjatse commented Nov 27, 2014

@soyuka You can check out the codes and year by yourself.
I think we'd better use the original one (high performance on linux), and deal with darwin system by ourself.

@soyuka
Copy link
Collaborator

soyuka commented Nov 27, 2014

isDarwin is not used (https://github.com/node-modules/treekill/blob/master/index.js)
https://github.com/pkrumins/node-tree-kill/blob/master/index.js has once() that has been removed in the @fengmk2 copy.

The only real change is in the spawned command :

var ps = spawn('ps', ['-o', 'pid', '--no-headers', '--ppid', parentPid]);

--no-headers will not work on darwin and the right command would be ps -e -o pid,ppid (that's the one used by @fengmk2 and the one we use here).

With the second command you don't specify the parentpid so it'll be slower anyway :/. Note I'd have to see if the darwin ps has a way to specify parent id, in this case it'd be interesting to refactor this.

Dunno what to do about this...

@Tjatse
Copy link
Collaborator Author

Tjatse commented Nov 28, 2014

I'll send a PR do do that with ps -eo pid,ppid | grep -w [PPID], could you help to review it?

It references to https://github.com/pkrumins/node-tree-kill/blob/master/index.js, we should have the professional ethics as a programer, right?

@Tjatse Tjatse mentioned this issue Nov 28, 2014
@Tjatse Tjatse closed this as completed Nov 28, 2014
@soyuka
Copy link
Collaborator

soyuka commented Nov 28, 2014

I'll test this on darwin this evening (GMT+1) ;)

@Tjatse
Copy link
Collaborator Author

Tjatse commented Nov 28, 2014

Appreciate, I've tested on MAC with v0.10 only....(GMT+8) x.-

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

No branches or pull requests

2 participants