Skip to content

src: turn inspector raw pointer into unique_ptr #16974

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
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 <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
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 <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
@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 <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@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 <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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 <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
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 <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@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 <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@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

@jasnell jasnell jasnell approved these changes

@cjihrig cjihrig cjihrig approved these changes

+2 more reviewers

@eugeneo eugeneo eugeneo approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

Reviewers whose approvals may not affect merge requirements

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