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

src: Remove lttng support. #18982

Closed
wants to merge 2 commits into from
Closed

src: Remove lttng support. #18982

wants to merge 2 commits into from

Conversation

GlenTiki
Copy link
Contributor

@GlenTiki GlenTiki commented Feb 24, 2018 •
edited
Loading

This cleans up and removes lttng support completely. Recent discussion
on a PR to deprecate lttng suggested that we remove it completely
pending feedback from the TSC.

This should be considered a non breaking change, as a recent PR reveals
that compiling with this system has been broken for nearly two years.

Refs: #18971
Refs: #18975
Refs: #18945

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

src, lib, lttng(?)

@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Feb 24, 2018
TimothyGu
TimothyGu approved these changes Feb 25, 2018
View reviewed changes
Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

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

👍 in spirit

lpinca
lpinca approved these changes Feb 25, 2018
View reviewed changes
deps/node-inspect/.eslintrc Outdated
LTTNG_HTTP_SERVER_RESPONSE: false
LTTNG_NET_SERVER_CONNECTION: false
LTTNG_NET_STREAM_END: false
DTRACE_NET_STREAM_END: false
Copy link
Member

Choose a reason for hiding this comment

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

Nit: missing new line

@AndreasMadsen AndreasMadsen added the semver-major PRs that contain breaking changes and should be released in the next major version. label Feb 25, 2018
@GlenTiki
src: Remove lttng support.
a170f7b 
This cleans up and removes lttng support completely. Recent discussion
on a PR to deprecate lttng suggested that we remove it completely
pending feedback from the TSC.

This should be considered a non breaking change, as a recent PR reveals
that compiling with this system has been broken for nearly two years.

Refs: nodejs#18971
Refs: nodejs#18975
Refs: nodejs#18945
@GlenTiki GlenTiki mentioned this pull request Feb 26, 2018
Closed
2 tasks
mcollina
mcollina approved these changes Feb 26, 2018
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM but it needs a deprecation notice

@GlenTiki
Copy link
Contributor Author

I'm looking to land this today - first time that I'm landing something with a deprecation notice included. Do I amend the deprecation notice commit when landing this?

@jasnell
Copy link
Member

jasnell commented Feb 28, 2018

@thekemkid ... whomever lands the PR should assign the DEP00XX code to whatever the next number is in the list and make sure that both the documentation and any in-code references to the number are updated.

@jasnell jasnell added this to the 10.0.0 milestone Mar 1, 2018
GlenTiki added a commit that referenced this pull request Mar 1, 2018
@GlenTiki
src: Remove lttng support.
30fd3d2 
This cleans up and removes lttng support completely. Recent discussion
on a PR to deprecate lttng suggested that we remove it completely
pending feedback from the TSC.

This should be considered a non breaking change, as a recent PR reveals
that compiling with this system has been broken for nearly two years.

Refs: #18971
Refs: #18975
Refs: #18945

PR-URL: #18982
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
GlenTiki added a commit that referenced this pull request Mar 1, 2018
@GlenTiki
doc: add a deprecation message for removing lttng
740c426 
PR-URL: #18982
Refs: #18971
Refs: #18975
Refs: #18945
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@GlenTiki
Copy link
Contributor Author

GlenTiki commented Mar 1, 2018

Landed in f7a6971...740c426

@GlenTiki GlenTiki closed this Mar 1, 2018
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@GlenTiki @MayaLekova
src: Remove lttng support.
fd24991 
This cleans up and removes lttng support completely. Recent discussion
on a PR to deprecate lttng suggested that we remove it completely
pending feedback from the TSC.

This should be considered a non breaking change, as a recent PR reveals
that compiling with this system has been broken for nearly two years.

Refs: nodejs#18971
Refs: nodejs#18975
Refs: nodejs#18945

PR-URL: nodejs#18982
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@GlenTiki @MayaLekova
doc: add a deprecation message for removing lttng
98660ee 
PR-URL: nodejs#18982
Refs: nodejs#18971
Refs: nodejs#18975
Refs: nodejs#18945
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott Trott mentioned this pull request Oct 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@JacksonTian JacksonTian JacksonTian approved these changes

@jasnell jasnell jasnell approved these changes

@evanlucas evanlucas evanlucas approved these changes

@addaleax addaleax addaleax approved these changes

@lpinca lpinca lpinca approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees
No one assigned
Labels
lib / src Issues and PRs related to general changes in the lib or src directory. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Milestone
10.0.0
Development

Successfully merging this pull request may close these issues.