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

doc: remove systemtap from tierlist #43605

Closed
wants to merge 1 commit into from

Conversation

tony-go
Copy link
Member

@tony-go tony-go commented Jun 28, 2022

Hey 👋

Context

The diagnostic working group currently works on an initiative to re-evaluate the diagnostic tooling list and its maturity.

Note: it's normally the last batch of updates before closing the issue.

Updated

In the previous instance, we examined the case of the Systemtap module: issue link.

Anyone in the instance has an idea about a potential usage in the field, @RafaelGSS used it a long time ago. Nevertheless, we don’t have anyone in charge of this support, we decided to remove it.

But in case you have a different opinion, please share your thoughts in this PR 🙏.

Discuss

As usual, feel free to share your thoughts on that and your experience with this tool.

With love ❤️

cc @nodejs/diagnostics

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/tsc

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jun 28, 2022
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM. I'm skeptical the systemtap integration is still in a functional state. The last update was four years ago in #20152 and it was consistently broken in the five years leading up to that. I'm inclined to simply remove it.

@tony-go
Copy link
Member Author

tony-go commented Jun 30, 2022

LGTM. I'm skeptical the systemtap integration is still in a functional state. The last update was four years ago in #20152 and it was consistently broken in the five years leading up to that. I'm inclined to simply remove it.

Thanks a lot for your feedback Ben ^^

@RafaelGSS RafaelGSS added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 30, 2022
@RafaelGSS RafaelGSS added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 1, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 1, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/43605
✔  Done loading data for nodejs/node/pull/43605
----------------------------------- PR info ------------------------------------
Title      doc: remove systemtap from tierlist (#43605)
Author     Tony Gorez  (@tony-go)
Branch     tony-go:remove-systemtap-tierlist -> nodejs:main
Labels     doc, author ready
Commits    1
 - doc: remove systemtap from tierlist
Committers 1
 - Tony Gorez 
PR-URL: https://github.com/nodejs/node/pull/43605
Reviewed-By: Ben Noordhuis 
Reviewed-By: Rafael Gonzaga 
Reviewed-By: Darshan Sen 
Reviewed-By: Gerhard Stöbich 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43605
Reviewed-By: Ben Noordhuis 
Reviewed-By: Rafael Gonzaga 
Reviewed-By: Darshan Sen 
Reviewed-By: Gerhard Stöbich 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Tue, 28 Jun 2022 19:28:47 GMT
   ✔  Approvals: 5
   ✔  - Ben Noordhuis (@bnoordhuis): https://github.com/nodejs/node/pull/43605#pullrequestreview-1023140598
   ✔  - Rafael Gonzaga (@RafaelGSS): https://github.com/nodejs/node/pull/43605#pullrequestreview-1023382076
   ✔  - Darshan Sen (@RaisinTen) (TSC): https://github.com/nodejs/node/pull/43605#pullrequestreview-1023409284
   ✔  - Gerhard Stöbich (@Flarna): https://github.com/nodejs/node/pull/43605#pullrequestreview-1023949009
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/43605#pullrequestreview-1026590703
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
   952cc0a038..574ad6d89d  main       -> origin/main
✔  origin/main is now up-to-date
main is out of sync with origin/main. Mismatched commits:
 - 03fa68b99e doc: remove dtrace from tierlist
 - 574ad6d89d doc: remove dtrace from tierlist
--------------------------------------------------------------------------------
HEAD is now at 574ad6d89d doc: remove dtrace from tierlist
   ✔  Reset to origin/main
- Downloading patch for 43605
From https://github.com/nodejs/node
 * branch                  refs/pull/43605/merge -> FETCH_HEAD
✔  Fetched commits as 2bd940c0cb59..d6e4f567c55f
--------------------------------------------------------------------------------
Auto-merging doc/contributing/diagnostic-tooling-support-tiers.md
CONFLICT (content): Merge conflict in doc/contributing/diagnostic-tooling-support-tiers.md
error: could not apply d6e4f567c5... doc: remove systemtap from tierlist
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
   ✖  Failed to apply patches
https://github.com/nodejs/node/actions/runs/2598812931

@RafaelGSS
Copy link
Member

@tony-go can you resolve conflicts?

@tony-go tony-go force-pushed the remove-systemtap-tierlist branch from d6e4f56 to 24400e6 Compare July 2, 2022 18:44
@tony-go
Copy link
Member Author

tony-go commented Jul 2, 2022

Rebased ↪️

lpinca pushed a commit that referenced this pull request Jul 3, 2022
PR-URL: #43605
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gerhard St<C3><B6>bich <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@lpinca
Copy link
Member

lpinca commented Jul 3, 2022

Landed in 627497b.

@lpinca lpinca closed this Jul 3, 2022
targos pushed a commit that referenced this pull request Jul 12, 2022
PR-URL: #43605
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gerhard St<C3><B6>bich <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Jul 20, 2022
PR-URL: #43605
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gerhard St<C3><B6>bich <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Jul 31, 2022
PR-URL: #43605
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gerhard St<C3><B6>bich <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
PR-URL: nodejs/node#43605
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Gerhard St<C3><B6>bich <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants