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

src: fix LTTNG tracepoint macros #9404

Closed
wants to merge 1 commit into from
Closed

Conversation

joshgav
Copy link
Contributor

@joshgav joshgav commented Nov 1, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

Description of change

Closing parentheses from TRACEPOINT_EVENT were removed accidentally in #7462, specifically f1d2792. As a result, building following ./configure --with-lttng fails.

This PR simply adds closing parentheses to the LTTNG tracepoint definitions to fix this.

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 1, 2016
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM if it works.

Added closing parentheses to LTTNG tracepoint definitions.

PR-URL: nodejs#9404
Reviewed-By: <tbd>
Reviewed-By: <tbd>
@joshgav
Copy link
Contributor Author

joshgav commented Nov 1, 2016

This builds now but the tracepoints don't show in lttng list --userspace. @nodejs/diagnostics any ideas?

@rvagg
Copy link
Member

rvagg commented Nov 1, 2016

pretty odd, probably @thekemkid's wheelhouse

@jasnell
Copy link
Member

jasnell commented Jan 11, 2017

Ping @nodejs/diagnostics

@GlenTiki
Copy link
Contributor

Oh crud, just seeing this now -> let me figure out what's up with lttng later and I will post findings. The code however lgtm

@jasnell
Copy link
Member

jasnell commented Mar 24, 2017

@joshgav ... is this still needed?

@fhinkel fhinkel added the stalled Issues and PRs that are stalled. label Mar 26, 2017
@joshgav
Copy link
Contributor Author

joshgav commented Apr 5, 2017

@jasnell

@joshgav ... is this still needed?

will need to pick it up again and test at some point, sorry can't confirm at the moment. feel free to close and I'll reopen when I get to it.

@joshgav
Copy link
Contributor Author

joshgav commented May 8, 2017

closing this since I don't know when I'll be able to pick it up again, will open a new issue at that point. Thanks!

@joshgav joshgav closed this May 8, 2017
@ChALkeR ChALkeR mentioned this pull request Feb 24, 2018
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. stalled Issues and PRs that are stalled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants