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.

Sign up for GitHub

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

Jump to bottom

build: correct minor typo in lttng help message #16101

Closed
wants to merge 2 commits into from
Closed

build: correct minor typo in lttng help message #16101

wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Oct 9, 2017

Currently the help message when using --with-lttng looks like this:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available to Linux)

This commit makes the help message consistent with the error message
that is raised if --with-lttng is used on a non-Linux operating
system:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available on Linux)
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

@danbev
build: correct minor typo in lttng help message
b10aef0 
Currently the help message when using --with-lttng looks like this:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available to Linux)

This commit makes the help message consistent with the error message
that is raised if --with-lttng is used on a non-Linux operating
system:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available on Linux)
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to build files or the CI. label Oct 9, 2017
@gibfahn
Copy link
Member

gibfahn commented Oct 9, 2017

If this is for consistency with

node/configure

Line 888 in 84579b1

raise Exception('lttng is only supported on Linux.')

why not change that line so it matches?

-raise Exception('lttng is only supported on Linux.') 
+raise Exception('Lttng is only available on Linux.')

@danbev
Copy link
Contributor Author

danbev commented Oct 10, 2017

If this is for consistency with

I was mainly thinking of the only supported to Linux which didn't sound right to me. I think the rest of the message is consistent with other flags like --with-etw.

@danbev
Copy link
Contributor Author

danbev commented Oct 11, 2017

@gibfahn Just wanted to double check this with you before I land it in regards to the comment above.

@gibfahn
Copy link
Member

gibfahn commented Oct 11, 2017

it just seems weird for the help to say:

build with Lttng (Only available on Linux)')

and the error to say:

lttng is only supported on Linux.

Why not make them both available or both supported?

@danbev
Copy link
Contributor Author

danbev commented Oct 11, 2017

Why not make them both available or both supported?

Ah I see now. I've updated it to supported. Thanks

danbev added a commit to danbev/node that referenced this pull request Oct 12, 2017
@danbev
build: correct minor typo in lttng help message
6cc4cf7 
Currently the help message when using --with-lttng looks like this:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available to Linux)

This commit makes the help message consistent with the error message
that is raised if --with-lttng is used on a non-Linux operating
system:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available on Linux)

PR-URL: nodejs#16101
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@danbev
Copy link
Contributor Author

danbev commented Oct 12, 2017

Landed in 6cc4cf7

@danbev danbev closed this Oct 12, 2017
@danbev danbev deleted the configure-lttng-typo branch October 12, 2017 09:04
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 15, 2017
@danbev @addaleax
build: correct minor typo in lttng help message
363e0ee 
Currently the help message when using --with-lttng looks like this:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available to Linux)

This commit makes the help message consistent with the error message
that is raised if --with-lttng is used on a non-Linux operating
system:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available on Linux)

PR-URL: nodejs/node#16101
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
targos pushed a commit that referenced this pull request Oct 18, 2017
@danbev @targos
build: correct minor typo in lttng help message
9bcb780 
Currently the help message when using --with-lttng looks like this:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available to Linux)

This commit makes the help message consistent with the error message
that is raised if --with-lttng is used on a non-Linux operating
system:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available on Linux)

PR-URL: #16101
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
targos pushed a commit that referenced this pull request Oct 18, 2017
@danbev @targos
build: correct minor typo in lttng help message
37ab447 
Currently the help message when using --with-lttng looks like this:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available to Linux)

This commit makes the help message consistent with the error message
that is raised if --with-lttng is used on a non-Linux operating
system:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available on Linux)

PR-URL: #16101
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 15, 2017
@danbev @MylesBorins
build: correct minor typo in lttng help message
c965349 
Currently the help message when using --with-lttng looks like this:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available to Linux)

This commit makes the help message consistent with the error message
that is raised if --with-lttng is used on a non-Linux operating
system:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available on Linux)

PR-URL: #16101
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
@danbev @MylesBorins
build: correct minor typo in lttng help message
bc27200 
Currently the help message when using --with-lttng looks like this:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available to Linux)

This commit makes the help message consistent with the error message
that is raised if --with-lttng is used on a non-Linux operating
system:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available on Linux)

PR-URL: #16101
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
@danbev @MylesBorins
build: correct minor typo in lttng help message
6d63612 
Currently the help message when using --with-lttng looks like this:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available to Linux)

This commit makes the help message consistent with the error message
that is raised if --with-lttng is used on a non-Linux operating
system:

$ ./configure --help | grep 'with-lttng'
  --with-lttng          build with Lttng (Only available on Linux)

PR-URL: #16101
Reviewed-By: Lance Ball <lball@redhat.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lance lance lance approved these changes

@gibfahn gibfahn gibfahn approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@danbev @gibfahn @lance @MylesBorins @nodejs-github-bot