Skip to content

src: use Maybe<void> where bool isn't needed#54575

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
targos:src-maybe-void
Sep 17, 2024
Merged

src: use Maybe<void> where bool isn't needed#54575
nodejs-github-bot merged 1 commit intonodejs:mainfrom
targos:src-maybe-void

Conversation

@targos
Copy link
Member

@targos targos commented Aug 26, 2024

No description provided.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 26, 2024

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Aug 26, 2024
@codecov
Copy link

codecov bot commented Aug 26, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 88.06%. Comparing base (53ede87) to head (d64b603).
Report is 517 commits behind head on main.

Files with missing lines Patch % Lines
src/node_contextify.cc 33.33% 4 Missing ⚠️
src/cares_wrap.cc 66.66% 1 Missing ⚠️
src/string_bytes.h 66.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #54575      +/-   ##
==========================================
- Coverage   88.07%   88.06%   -0.01%     
==========================================
  Files         652      652              
  Lines      183542   183542              
  Branches    35862    35858       -4     
==========================================
- Hits       161653   161645       -8     
- Misses      15144    15149       +5     
- Partials     6745     6748       +3
Files with missing lines Coverage Δ
src/node_contextify.h 73.33% <ø> (ø)
src/cares_wrap.cc 65.44% <66.66%> (ø)
src/string_bytes.h 77.77% <66.66%> (ø)
src/node_contextify.cc 81.35% <33.33%> (+0.08%) ⬆️

... and 22 files with indirect coverage changes

@lpinca lpinca added request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Aug 29, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 29, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 29, 2024

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 30, 2024

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Aug 31, 2024

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 1, 2024

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 2, 2024

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

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 2, 2024

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

@targos
Copy link
Member Author

targos commented Sep 16, 2024

Rebased

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 16, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 16, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 16, 2024

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

@targos
Copy link
Member Author

targos commented Sep 17, 2024

CI passed. Can I have another review to land?

@targos targos added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Sep 17, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 17, 2024
@nodejs-github-bot nodejs-github-bot merged commit d2a4f92 into nodejs:main Sep 17, 2024
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 17, 2024

Landed in d2a4f92

@targos targos deleted the src-maybe-void branch September 17, 2024 07:57
targos added a commit that referenced this pull request Oct 4, 2024
@targos
src: use Maybe<void> where bool isn't needed
f332c4c 
PR-URL: #54575
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@aduh95 aduh95 mentioned this pull request Oct 9, 2024
Merged
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
@targos @louwers
src: use Maybe<void> where bool isn't needed
7060b7f 
PR-URL: nodejs#54575
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@marco-ippolito marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Nov 16, 2024
tpoisseau pushed a commit to tpoisseau/node that referenced this pull request Nov 21, 2024
@targos @tpoisseau
src: use Maybe<void> where bool isn't needed
9e55ed0 
PR-URL: nodejs#54575
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
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

@lpinca lpinca lpinca approved these changes

@tniessen tniessen tniessen approved these changes

@JungMinu JungMinu JungMinu approved these changes

@legendecas legendecas legendecas approved these changes

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. lib / src Issues and PRs related to general changes in the lib or src directory. 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.

8 participants

@targos @nodejs-github-bot @jasnell @lpinca @tniessen @JungMinu @legendecas @marco-ippolito