rename interceptors appripriately #600
Conversation
Summary of ChangesHello @pitabwire, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing code clarity and maintainability by renaming the generic Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request appropriately renames Interceptor to ValidationInterceptor for better clarity, along with related functions and types. This improves the descriptiveness of the validation logic. My review focuses on some side effects of what appears to be a broad search-and-replace, which has led to some incorrect comments. I've also suggested renaming test functions for consistency and pointed out a few minor typos. Additionally, please note the typo in the pull request title ('appripriately' should be 'appropriately').
| ) | ||
|
|
||
| // AuthInterceptor implements connect.Interceptor for JWT authentication. | ||
| // AuthInterceptor implements connect.ValidationInterceptor for JWT authentication. |
There was a problem hiding this comment.
This comment is incorrect. AuthInterceptor implements the connect.Interceptor interface. ValidationInterceptor is a different struct in this package that also implements connect.Interceptor. This change appears to be an artifact of a search-and-replace operation and should be reverted to refer to connect.Interceptor to avoid confusion.
| // AuthInterceptor implements connect.ValidationInterceptor for JWT authentication. | |
| // AuthInterceptor implements connect.Interceptor for JWT authentication. |
|
|
||
| // WrapUnary implements connect.Interceptor. | ||
| func (i *Interceptor) WrapUnary(next connect.UnaryFunc) connect.UnaryFunc { | ||
| // WrapUnary implements connect.ValidationInterceptor. |
There was a problem hiding this comment.
This comment is misleading. WrapUnary is a method that helps ValidationInterceptor satisfy the connect.Interceptor interface. It does not implement connect.ValidationInterceptor. This seems to be an error from a search-and-replace. Please revert it to refer to connect.Interceptor.
| // WrapUnary implements connect.ValidationInterceptor. | |
| // WrapUnary implements connect.Interceptor. |
|
|
||
| // WrapStreamingClient implements connect.Interceptor. | ||
| func (i *Interceptor) WrapStreamingClient(next connect.StreamingClientFunc) connect.StreamingClientFunc { | ||
| // WrapStreamingClient implements connect.ValidationInterceptor. |
There was a problem hiding this comment.
This comment is misleading. WrapStreamingClient is a method that helps ValidationInterceptor satisfy the connect.Interceptor interface. It does not implement connect.ValidationInterceptor. This seems to be an error from a search-and-replace. Please revert it to refer to connect.Interceptor.
| // WrapStreamingClient implements connect.ValidationInterceptor. | |
| // WrapStreamingClient implements connect.Interceptor. |
|
|
||
| // WrapStreamingHandler implements connect.Interceptor. | ||
| func (i *Interceptor) WrapStreamingHandler(next connect.StreamingHandlerFunc) connect.StreamingHandlerFunc { | ||
| // WrapStreamingHandler implements connect.ValidationInterceptor. |
There was a problem hiding this comment.
This comment is misleading. WrapStreamingHandler is a method that helps ValidationInterceptor satisfy the connect.Interceptor interface. It does not implement connect.ValidationInterceptor. This seems to be an error from a search-and-replace. Please revert it to refer to connect.Interceptor.
| // WrapStreamingHandler implements connect.ValidationInterceptor. | |
| // WrapStreamingHandler implements connect.Interceptor. |
| ) | ||
|
|
||
| // An Option configures an [Interceptor]. | ||
| // An Option configures an [ValidationInterceptor]. |
There was a problem hiding this comment.
There's a grammatical error here. The article should be "a" instead of "an" before "ValidationInterceptor".
| // An Option configures an [ValidationInterceptor]. | |
| // An Option configures a [ValidationInterceptor]. |
| } | ||
|
|
||
| // WithValidateResponses configures the [Interceptor] to also validate reponses | ||
| // WithValidateResponses configures the [ValidationInterceptor] to also validate reponses |
There was a problem hiding this comment.
There is a typo in "reponses". It should be "responses".
| // WithValidateResponses configures the [ValidationInterceptor] to also validate reponses | |
| // WithValidateResponses configures the [ValidationInterceptor] to also validate responses |
| } | ||
|
|
||
| // NewInterceptor builds an Interceptor. The default configuration is | ||
| // NewValidationInterceptor builds an ValidationInterceptor. The default configuration is |
There was a problem hiding this comment.
There's a grammatical error here. The article should be "a" instead of "an" before "ValidationInterceptor".
| // NewValidationInterceptor builds an ValidationInterceptor. The default configuration is | |
| // NewValidationInterceptor builds a ValidationInterceptor. The default configuration is |
| @@ -18,31 +18,31 @@ import ( | |||
|
|
|||
| func TestNewInterceptor(t *testing.T) { | |||
There was a problem hiding this comment.
For consistency with the function being tested (NewValidationInterceptor), this test function should be renamed to TestNewValidationInterceptor.
| func TestNewInterceptor(t *testing.T) { | |
| func TestNewValidationInterceptor(t *testing.T) { |
| @@ -378,7 +378,7 @@ func BenchmarkValidateAllStructs(b *testing.B) { | |||
| } | |||
|
|
|||
| func BenchmarkInterceptorWrapUnary(b *testing.B) { | |||
There was a problem hiding this comment.
For consistency with the renaming of Interceptor to ValidationInterceptor, this benchmark function should be renamed to BenchmarkValidationInterceptorWrapUnary.
| func BenchmarkInterceptorWrapUnary(b *testing.B) { | |
| func BenchmarkValidationInterceptorWrapUnary(b *testing.B) { |
1 participant
No description provided.