Skip to content

Use grpc_error defs instead of NULL#19572

Merged
nanahpang merged 1 commit intogrpc:masterfrom
legendtang:errfix
Jul 11, 2019
Merged

Use grpc_error defs instead of NULL#19572
nanahpang merged 1 commit intogrpc:masterfrom
legendtang:errfix

Conversation

@legendtang
Copy link
Copy Markdown
Contributor

@legendtang legendtang commented Jul 6, 2019

NULL is not clear for a grpc_error. Not sure we should use other error
codes here.

@thelinuxfoundation
Copy link
Copy Markdown

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

Regards,
CLA GitHub bot

NULL is not clear for a grpc_error. Not sure we should use other error
codes here.
@legendtang legendtang changed the title Use grpc_error defs; Fix NPDs; Use grpc_error defs instead of NULL Jul 10, 2019
@a11r a11r requested a review from nanahpang July 10, 2019 17:21
@nanahpang nanahpang added lang/c++ release notes: no Indicates if PR should not be in release notes kokoro:force-run labels Jul 11, 2019
Copy link
Copy Markdown
Contributor

@nanahpang nanahpang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree that we should change it to GRPC_ERROR_NONE even though they are the same thing here. For readability and avoiding errors when GRPC_ERROR_NONE is assigned to another value, it is better to clarify it now. Thanks for pointing this out. LGTM

@nanahpang nanahpang merged commit 7044945 into grpc:master Jul 11, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lang/c++ release notes: no Indicates if PR should not be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants