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,build: add no user defined deduction guides of CTAD check #56071

Merged
merged 1 commit into from
Dec 6, 2024
Merged

src,build: add no user defined deduction guides of CTAD check #56071

merged 1 commit into from
Dec 6, 2024

Conversation

legendecas
Copy link
Member

To our secondary C++ style guide:

Do not use CTAD with a given template unless the template's maintainers have opted into supporting use of CTAD by providing at least one explicit deduction guide (all templates in the std namespace are also presumed to have opted in). This should be enforced with a compiler warning if available.

Ref: https://google.github.io/styleguide/cppguide.html#CTAD

To use CTAD with a template, it must be defined with at least one deduction guide (std templates are defined with guides). Enforcing this rule with compiler checks.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Nov 29, 2024
@legendecas legendecas mentioned this pull request Nov 29, 2024
Merged
@legendecas legendecas changed the title build: add no user defined deduction guides of CTAD check src,build: add no user defined deduction guides of CTAD check Nov 29, 2024
@codecov Codecov
Copy link

codecov bot commented Nov 29, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 88.01%. Comparing base (d09458f) to head (8d59040).
Report is 49 commits behind head on main.

Files with missing lines Patch % Lines
src/quic/session.cc 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #56071      +/-   ##
==========================================
+ Coverage   87.96%   88.01%   +0.04%     
==========================================
  Files         656      656              
  Lines      188384   188988     +604     
  Branches    35968    35989      +21     
==========================================
+ Hits       165707   166331     +624     
- Misses      15845    15846       +1     
+ Partials     6832     6811      -21
Files with missing lines Coverage Δ
src/quic/session.cc 5.35% <0.00%> (ø)

... and 31 files with indirect coverage changes

@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Nov 29, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 29, 2024
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@legendecas legendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 6, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 6, 2024
@nodejs-github-bot nodejs-github-bot merged commit 4211ab5 into nodejs:main Dec 6, 2024
63 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 4211ab5

@legendecas legendecas deleted the build/ctad branch December 6, 2024 10:05
@legendecas legendecas added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. and removed dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Dec 6, 2024
targos pushed a commit that referenced this pull request Dec 6, 2024
@legendecas @targos
src,build: add no user defined deduction guides of CTAD check
8a93548 
PR-URL: #56071
Refs: https://google.github.io/styleguide/cppguide.html#CTAD
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Dec 6, 2024
Merged
aduh95 pushed a commit that referenced this pull request Dec 13, 2024
@legendecas @aduh95
src,build: add no user defined deduction guides of CTAD check
fdd7a20 
PR-URL: #56071
Refs: https://google.github.io/styleguide/cppguide.html#CTAD
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Dec 13, 2024
@legendecas @aduh95
src,build: add no user defined deduction guides of CTAD check
9fcea1b 
PR-URL: #56071
Refs: https://google.github.io/styleguide/cppguide.html#CTAD
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Dec 13, 2024
@legendecas @aduh95
src,build: add no user defined deduction guides of CTAD check
1377528 
PR-URL: #56071
Refs: https://google.github.io/styleguide/cppguide.html#CTAD
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this pull request Dec 18, 2024
@legendecas @aduh95
src,build: add no user defined deduction guides of CTAD check
0b1b6fd 
PR-URL: #56071
Refs: https://google.github.io/styleguide/cppguide.html#CTAD
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ruyadorno ruyadorno mentioned this pull request Dec 20, 2024
Merged
ruyadorno pushed a commit that referenced this pull request Jan 5, 2025
@legendecas @ruyadorno
src,build: add no user defined deduction guides of CTAD check
359fff1 
PR-URL: #56071
Refs: https://google.github.io/styleguide/cppguide.html#CTAD
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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

@anonrig anonrig anonrig approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@legendecas @nodejs-github-bot @jasnell @anonrig