Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

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

@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
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
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 added a commit that referenced this pull request Nov 17, 2017
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
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
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
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
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.