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

Default --prefix to "name" when --names is set #97

Merged
merged 3 commits into from Mar 21, 2017

Conversation

enaeseth
Copy link
Contributor

@enaeseth enaeseth commented Mar 4, 2017

I found it confusing that specifying --names without changing the --prefix continues to use a default prefix of index. This PR changes the default prefix to be name when --names is set and index when it's not.

It also cleans up the test code; since this adds new tests that use readline, I moved that functionality into tests/utils.js.

(cf: kimmobrunfeldt/concurrently#53)

- Consolidate readline test code in test/utils.js
- Move responsibility for DEBUG_TESTS to run()
- Remove unused cwd option
@gustavohenke gustavohenke self-requested a review March 7, 2017 14:38
src/main.js Outdated
@@ -81,7 +83,7 @@ function parseArgs() {
'-p, --prefix <prefix>',
'prefix used in logging for each process.\n' +
'Possible values: index, pid, time, command, name, none, or a template. Default: ' +
config.prefix + '. Example template: "{time}-{pid}"\n'
'index. Example template: "{time}-{pid}"\n'
Copy link
Member

Choose a reason for hiding this comment

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

What if we mentioned both defaults? Too fancy?

@gustavohenke gustavohenke merged commit 2614c5b into open-cli-tools:master Mar 21, 2017
@alecmev
Copy link

alecmev commented May 21, 2017

@gustavohenke You should release this 🙂 Took me a couple of minutes to realize that the latest release is older than this PR.

@RichDonnellan
Copy link

@gustavohenke I agree with @jeremejevs. I spent far too much time debugging only to realize that master isn't the same as the 3.4.0 release. 😕

@gustavohenke
Copy link
Member

@RichDonnellan @jeremejevs @enaeseth this was just published as v3.5.0.
I'm so sorry for taking this long to publish, folks!

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.

None yet

4 participants