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: turn inspector raw pointer into unique_ptr #16974

Closed
wants to merge 1 commit into from
Closed

src: turn inspector raw pointer into unique_ptr #16974

wants to merge 1 commit into from

Conversation

fhinkel
Copy link
Member

@fhinkel fhinkel commented Nov 12, 2017

Use a unique pointer to make ownership clear.

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

src

@fhinkel
src: turn inspector raw pointer into unique_ptr
f3ba11c 
Use a unique pointer to make ownership clear.
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 12, 2017
@fhinkel
Copy link
Member Author

fhinkel commented Nov 13, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/11404/

@fhinkel
Copy link
Member Author

fhinkel commented Nov 14, 2017

Landed in c07d127.

@fhinkel fhinkel closed this Nov 14, 2017
fhinkel added a commit that referenced this pull request Nov 14, 2017
@fhinkel
src: turn inspector raw pointer into unique_ptr
c07d127 
Use a unique pointer to make ownership clear.

PR-URL: #16974
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
evanlucas pushed a commit that referenced this pull request Nov 14, 2017
@fhinkel @evanlucas
src: turn inspector raw pointer into unique_ptr
71181b9 
Use a unique pointer to make ownership clear.

PR-URL: #16974
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
@fhinkel fhinkel mentioned this pull request Nov 15, 2017
Closed
2 tasks
fhinkel added a commit that referenced this pull request Nov 17, 2017
@fhinkel
doc: mention smart pointers in Cpp style guide
0e1abb1 
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins
Copy link
Contributor

Does this make sense for 6.x or 8.x LTS? It lands cleanly on 8.x but will require a backport for 6.x

@fhinkel
Copy link
Member Author

fhinkel commented Nov 17, 2017

I'd say let's not land the smart pointer changes.

@bnoordhuis
Copy link
Member

That's going to make future back-ports more difficult though.

@fhinkel
Copy link
Member Author

fhinkel commented Nov 18, 2017

Makes sense. Changing the labels

MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@fhinkel @MylesBorins
doc: mention smart pointers in Cpp style guide
03803ee 
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 13, 2017
@fhinkel @gibfahn
src: turn inspector raw pointer into unique_ptr
b1802ed 
Use a unique pointer to make ownership clear.

PR-URL: #16974
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Eugene Ostroukhov <[email protected]>
gibfahn pushed a commit that referenced this pull request Dec 19, 2017
@fhinkel @gibfahn
doc: mention smart pointers in Cpp style guide
f30fcdb 
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Closed
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@fhinkel @gibfahn
doc: mention smart pointers in Cpp style guide
706aab9 
Add rule for smart pointers, i.e., std::unique_ptr and std::shared_ptr,
to the Cpp style guide. Mostly copied from the Google style guide.

PR-URL: #17055
Ref: #16970
Ref: #16974
Ref: #17000
Ref: #17012
Ref: #17020
Ref: #17030
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@jasnell jasnell jasnell approved these changes

@eugeneo eugeneo eugeneo approved these changes

Assignees
No one assigned
Labels
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.
9 participants
@fhinkel @MylesBorins @bnoordhuis @eugeneo @jasnell @TimothyGu @cjihrig @gibfahn @nodejs-github-bot