GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — src: remove unused libraries by juanarbol · Pull Request #30827 · nodejs/node

Conversation

@juanarbol

@juanarbol juanarbol commented Dec 6, 2019

Copy link
Copy Markdown
Member
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 6, 2019
addaleax
addaleax approved these changes Dec 6, 2019
View reviewed changes
gengjiawen
gengjiawen approved these changes Dec 7, 2019
View reviewed changes
JungMinu
JungMinu approved these changes Dec 7, 2019
View reviewed changes
cjihrig
cjihrig approved these changes Dec 7, 2019
View reviewed changes
devnexen
devnexen approved these changes Dec 7, 2019
View reviewed changes
trivikr
trivikr approved these changes Dec 7, 2019
View reviewed changes
@trivikr trivikr added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Dec 7, 2019
@nodejs-github-bot

nodejs-github-bot commented Dec 7, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27460/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Trott
Trott approved these changes Dec 7, 2019
View reviewed changes
antsmartian
antsmartian approved these changes Dec 7, 2019
View reviewed changes
BridgeAR
BridgeAR approved these changes Dec 8, 2019
View reviewed changes
tniessen
tniessen previously approved these changes Dec 8, 2019
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Dec 8, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27505/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Dec 9, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27511/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TimothyGu

TimothyGu commented Dec 9, 2019

Copy link
Copy Markdown
Member

Hmm, it appears that at least algorithm and memory are used, since the file explicitly references unique_ptr and find_if. Is there some context I'm missing?
juanarbol reacted with thumbs up emoji juanarbol reacted with eyes emoji
All reactions
  • 👍 1 reaction
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@BridgeAR BridgeAR removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Dec 10, 2019
@juanarbol

juanarbol commented Dec 11, 2019

Copy link
Copy Markdown
Member Author

Sorry, I was based if it compiles, it works or something, so, I just saw a file where inclues specifies the used methods, I did the same here. Thanks @TimothyGu. I'll spend some time searching why this compiles.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tniessen
tniessen dismissed their stale review December 11, 2019 05:21

Does not apply anymore

@Trott Trott added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Dec 13, 2019
@nodejs-github-bot

nodejs-github-bot commented Dec 13, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27651/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott

Trott commented Dec 14, 2019

Copy link
Copy Markdown
Member

The C++ linter is flagging some issues with whitespace. make lint-cpp (or, on Windows, vcbuild lint-cpp) to see the results.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Dec 14, 2019
richardlau
richardlau previously requested changes Dec 14, 2019
View reviewed changes

@richardlau richardlau left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Commit message is no longer accurate.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@juanarbol
src: list used functions on headers
9114104
@juanarbol
juanarbol force-pushed the cc-unused-libs branch from 9dcdf88 to 9114104 Compare December 17, 2019 15:14
@richardlau
richardlau dismissed their stale review December 17, 2019 15:27

Commit message has been updated.

@nodejs-github-bot

nodejs-github-bot commented Dec 17, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27769/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Dec 17, 2019
@nodejs-github-bot

nodejs-github-bot commented Dec 18, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/27774/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
jasnell
jasnell approved these changes Dec 18, 2019
View reviewed changes
@BridgeAR

BridgeAR commented Dec 20, 2019

Copy link
Copy Markdown
Member

Landed in db9539b 🎉
juanarbol reacted with thumbs up emoji juanarbol reacted with hooray emoji
All reactions
  • 👍 1 reaction
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
BridgeAR pushed a commit that referenced this pull request Dec 20, 2019
@juanarbol @BridgeAR
src: list used functions on headers
db9539b 
PR-URL: #30827
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BridgeAR BridgeAR closed this Dec 20, 2019
BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
@juanarbol @BridgeAR
src: list used functions on headers
6b65caf 
PR-URL: #30827
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
v13.6.0 proposal #31238
Merged
targos pushed a commit that referenced this pull request Jan 14, 2020
@juanarbol @targos
src: list used functions on headers
3c63af2 
PR-URL: #30827
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@targos targos mentioned this pull request Jan 15, 2020
v12.15.0 release proposal #31368
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
@juanarbol @BethGriggs
src: list used functions on headers
f71b09f 
PR-URL: #30827
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anto Aravinth <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
v12.16.0 proposal #31691
Merged
@juanarbol
juanarbol deleted the cc-unused-libs branch January 19, 2021 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@Trott Trott Trott approved these changes
@addaleax addaleax addaleax approved these changes
@cjihrig cjihrig cjihrig approved these changes
@gengjiawen gengjiawen gengjiawen approved these changes
@JungMinu JungMinu JungMinu approved these changes
@BridgeAR BridgeAR BridgeAR approved these changes
@trivikr trivikr trivikr approved these changes
@tniessen tniessen tniessen left review comments
@richardlau richardlau richardlau left review comments
+2 more reviewers
@antsmartian antsmartian antsmartian approved these changes
@devnexen devnexen devnexen approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

15 participants

@juanarbol @nodejs-github-bot @TimothyGu @Trott @BridgeAR @jasnell @addaleax @antsmartian @cjihrig @tniessen @gengjiawen @devnexen @JungMinu @richardlau @trivikr
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.