FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: use ARES_SUCCESS instead of 0 by illusi0n7 · Pull Request #48834 · nodejs/node · GitHub

/ node Public

src: use ARES_SUCCESS instead of 0 - #48834

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
illusi0n7:dev/refactoring
Jul 23, 2023
ConversationCommits (1)ChecksFiles changed
Merged

src: use ARES_SUCCESS instead of 0#48834
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
illusi0n7:dev/refactoring

Conversation

illusi0n7 commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

Since error messages are also using defined values, it is more clear to use the defined value even when the operation succeeds.

src: use ARES_SUCCESS instead of 0
7b4fa6e 
Since error messages are also using defined values, it is more clear
to use the defined value even when the operation succeeds.

nodejs-github-bot commented Jul 19, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. needs-ci PRs that need a full CI run. labels Jul 19, 2023

ShogunPanda commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

Is ARES_SUCCESS defined as 0?

illusi0n7 commented Jul 19, 2023

Copy link
Copy Markdown
Contributor Author

sure. like this.

// ares.h
#define ARES_SUCCESS 0

ShogunPanda commented Jul 19, 2023

Copy link
Copy Markdown
Contributor

I guess I'm fine with it then!

illusi0n7 commented Jul 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

And return value is compared with ARES_SUCCESS in cares_wrap.h

//cares_wrap.h
status = Traits::Parse(this, response_data_);
if (status != ARES_SUCCESS)
  ParseError(status);

ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 19, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 19, 2023

This comment was marked as outdated.

Sign in to view

nodejs-github-bot commented Jul 19, 2023

Copy link
Copy Markdown
Collaborator

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

github-actions Bot mentioned this pull request Jul 20, 2023
Open
22 tasks
daeyeon added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jul 20, 2023
github-actions Bot mentioned this pull request Jul 21, 2023
Open
16 tasks
This was referenced Jul 22, 2023
Open
Open
lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 23, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 23, 2023
nodejs-github-bot merged commit 841b29c into nodejs:main Jul 23, 2023

nodejs-github-bot commented Jul 23, 2023

Copy link
Copy Markdown
Collaborator

Landed in 841b29c

This was referenced Jul 24, 2023
Open
Open
Open
github-actions Bot mentioned this pull request Jul 27, 2023
Open
18 tasks
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Jul 27, 2023
src: use ARES_SUCCESS instead of 0
693aff9 
Since error messages are also using defined values, it is more clear
to use the defined value even when the operation succeeds.

PR-URL: nodejs#48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
pluris mentioned this pull request Aug 7, 2023
Merged
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
src: use ARES_SUCCESS instead of 0
6ef2899 
Since error messages are also using defined values, it is more clear
to use the defined value even when the operation succeeds.

PR-URL: nodejs#48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
src: use ARES_SUCCESS instead of 0
2611b2c 
Since error messages are also using defined values, it is more clear
to use the defined value even when the operation succeeds.

PR-URL: nodejs#48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this pull request Aug 14, 2023
src: use ARES_SUCCESS instead of 0
eb300a5 
Since error messages are also using defined values, it is more clear
to use the defined value even when the operation succeeds.

PR-URL: nodejs#48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
RafaelGSS pushed a commit that referenced this pull request Aug 15, 2023
src: use ARES_SUCCESS instead of 0
578c3d1 
Since error messages are also using defined values, it is more clear
to use the defined value even when the operation succeeds.

PR-URL: #48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
UlisesGascon mentioned this pull request Aug 15, 2023
Merged
nodejs-github-bot pushed a commit that referenced this pull request Aug 20, 2023
src: use ARES_SUCCESS instead of 0
982e7a6 
PR-URL: #49048
Refs: #48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
src: use ARES_SUCCESS instead of 0
f2552a4 
PR-URL: #49048
Refs: #48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
targos pushed a commit that referenced this pull request Nov 27, 2023
src: use ARES_SUCCESS instead of 0
67dba57 
Since error messages are also using defined values, it is more clear
to use the defined value even when the operation succeeds.

PR-URL: #48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
targos pushed a commit that referenced this pull request Nov 27, 2023
src: use ARES_SUCCESS instead of 0
158c91a 
PR-URL: #49048
Refs: #48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
targos mentioned this pull request Nov 28, 2023
Merged
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
src: use ARES_SUCCESS instead of 0
d43a85e 
Since error messages are also using defined values, it is more clear
to use the defined value even when the operation succeeds.

PR-URL: nodejs/node#48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
src: use ARES_SUCCESS instead of 0
534aa0e 
PR-URL: nodejs/node#49048
Refs: nodejs/node#48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
src: use ARES_SUCCESS instead of 0
208d856 
Since error messages are also using defined values, it is more clear
to use the defined value even when the operation succeeds.

PR-URL: nodejs/node#48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
src: use ARES_SUCCESS instead of 0
59cf0e0 
PR-URL: nodejs/node#49048
Refs: nodejs/node#48834
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Deokjin Kim <deokjin81.kim@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ShogunPanda ShogunPanda approved these changes
lpinca lpinca approved these changes
JungMinu JungMinu approved these changes
deokjinkim deokjinkim approved these changes
daeyeon daeyeon approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. 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.

7 participants

Back | FazBrowse Home | New Git URL