Skip to content

Conversation

@nguymin4
Copy link
Contributor

@nguymin4 nguymin4 commented Sep 24, 2017

At the moment when we run several gulp tasks separately, they have the same process title gulp which is very hard to differentiate and monitor them. This PR is to solve that issue.

@phated
Copy link
Member

phated commented Sep 25, 2017

Thanks for submitting this here.

var cmdArgs = process.argv.slice(2);
var cli = new Liftoff({
name: 'gulp',
processTitle: ['gulp'].concat(cmdArgs).join(' '),
Copy link
Member

Choose a reason for hiding this comment

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

I think we can make the process title nicer. Maybe we should only show the tasks being run? Maybe we should wrap them in ( )

Copy link
Contributor Author

@nguymin4 nguymin4 Sep 25, 2017

Choose a reason for hiding this comment

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

I can wrap them in ( ) but I think we should keep the original command argv without any modification. So for example gulp watch should be different from gulp watch -L. Is that ok for you?

Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit concerned that the process title will get too long. Maybe it should get truncated?

Copy link
Member

Choose a reason for hiding this comment

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

@sttk any ideas about how we can reduce the size of the process title but still show uniqueness across flags?

Copy link
Contributor

Choose a reason for hiding this comment

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

@phated I have no idea to make short it.

I agree with @nguymin4's saying, keeping the original command argv, but there may be some platforms or tools which are not support too long process name. (Though I don't know actual cases.)

I think it would be better that this feature is optional and able to be specified in config file.

Copy link
Contributor Author

@nguymin4 nguymin4 Oct 4, 2017

Choose a reason for hiding this comment

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

Actually, I cannot come up with a case that the command is too long. If it is, then you are doing something wrong, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

@nguymin4 My thought seemed to be overcautious. I think no problem about this now.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, if @sttk doesn't see a problem with it. Can you turn this into a named utility function?

@phated
Copy link
Member

phated commented Dec 20, 2017

Rebased as e5fdefb - Thanks @nguymin4

@phated phated closed this Dec 20, 2017
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.

3 participants