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.

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

codes: update docstring to indicate expected usage #6701

Merged
merged 2 commits into from
Oct 10, 2023

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented Oct 6, 2023

Related to grpc/grpc#34588; specifically document that users should use ONLY the values defined by us.

RELEASE NOTES:

  • codes: clarify that only codes defined by this package are valid and that users should not cast other values to codes.Code

@dfawley dfawley added the Type: Documentation Documentation or examples label Oct 6, 2023
@dfawley dfawley added this to the 1.60 Release milestone Oct 6, 2023
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #6701 (9b61d75) into master (afaf31a) will decrease coverage by 0.10%.
Report is 3 commits behind head on master.
The diff coverage is n/a.

Additional details and impacted files

@zasweq zasweq self-requested a review October 10, 2023 17:34
@zasweq zasweq self-assigned this Oct 10, 2023
Copy link
Contributor

@zasweq zasweq left a comment

Choose a reason for hiding this comment

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

LGTM.

codes/codes.go Outdated
Comment on lines 33 to 34
// other code values. Behavior and interopability are implementation-specific
// and not guaranteed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you clarify the scope of the language of "behavior and interoperability" here? What is not guaranteed? Any permutation of languages with defined status codes? Only status codes undefined?

Copy link
Member Author

Choose a reason for hiding this comment

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

Clarified.

codes/codes.go Outdated
// A Code is an unsigned 32-bit error code as defined in the gRPC spec.
// A Code is a status code according to the gRPC spec:
//
// https://github.com/grpc/grpc/blob/master/doc/statuscodes.md
Copy link
Contributor

Choose a reason for hiding this comment

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

Easwar made me switch urls to a certain format: https://github.com/grpc/grpc-go/blob/master/xds/internal/balancer/wrrlocality/balancer.go#L19 (I think this is a strong heuristic of .go files). Can you please switch this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@zasweq zasweq assigned dfawley and unassigned zasweq Oct 10, 2023
@dfawley dfawley merged commit cb3ae76 into grpc:master Oct 10, 2023
13 checks passed
@dfawley dfawley deleted the codesDoc branch October 10, 2023 20:06
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Documentation Documentation or examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants