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

Fix revive identified linter issues: var-declaration, indent-error-flow, increment-decrement, superfluous-else #7528

Merged
merged 7 commits into from
Aug 23, 2024

Conversation

janardhanvissa
Copy link
Contributor

@janardhanvissa janardhanvissa commented Aug 18, 2024

Partially addresses #7444

Fixed below linter issues
- 6 var-declaration
- 3 indent-error-flow
- 2 increment-decrement
- 1 superfluous-else

RELEASE NOTES: None

janardhanvissa and others added 6 commits August 14, 2024 15:06
Optimising the code by fixing var-declaration, indent-error-flow, increment-decrement, superfluous-else
Revert "Optimising the code by fixing var-declaration, indent-error-flow, increment-decrement, superfluous-else"
Copy link

codecov bot commented Aug 18, 2024

Codecov Report

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

Project coverage is 81.93%. Comparing base (ec9dff7) to head (fb38c64).
Report is 46 commits behind head on master.

Files Patch % Lines
profiling/cmd/remote.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7528      +/-   ##
==========================================
+ Coverage   81.56%   81.93%   +0.36%     
==========================================
  Files         354      360       +6     
  Lines       27076    27535     +459     
==========================================
+ Hits        22085    22560     +475     
+ Misses       3802     3782      -20     
- Partials     1189     1193       +4     
Files Coverage Δ
internal/channelz/funcs.go 100.00% <ø> (ø)
internal/transport/http2_client.go 92.09% <100.00%> (+0.03%) ⬆️
internal/transport/http_util.go 93.30% <ø> (+6.89%) ⬆️
profiling/cmd/remote.go 0.00% <0.00%> (ø)

... and 73 files with indirect coverage changes

@janardhanvissa janardhanvissa changed the title Godoclinter Optimising the code by fixing var-declaration, indent-error-flow, increment-decrement, superfluous-else Aug 18, 2024
@janardhanvissa janardhanvissa changed the title Optimising the code by fixing var-declaration, indent-error-flow, increment-decrement, superfluous-else Optimizing the code by fixing var-declaration, indent-error-flow, increment-decrement, superfluous-else Aug 18, 2024
@purnesh42H purnesh42H changed the title Optimizing the code by fixing var-declaration, indent-error-flow, increment-decrement, superfluous-else Fix revive identified linter issues: var-declaration, indent-error-flow, increment-decrement, superfluous-else Aug 18, 2024
@purnesh42H purnesh42H added this to the 1.67 Release milestone Aug 18, 2024
@purnesh42H purnesh42H self-requested a review August 18, 2024 18:11
@purnesh42H purnesh42H added the Type: Internal Cleanup Refactors, etc label Aug 18, 2024
Copy link
Contributor

@purnesh42H purnesh42H left a comment

Choose a reason for hiding this comment

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

lgtm

@arvindbr8
Copy link
Member

Thanks for taking a stab at this @janardhanvissa -- However I would prefer if these fixes also have an effect in our CI. I have #7543 in review.

Once we merge it, we can then re-enable the checks which are fixed in this PR

@arvindbr8
Copy link
Member

I spoke to @purnesh42H offline and seems like @janardhanvissa is working on all the remaining fixes (expect for unused-parameters) at the moment. Seems like it's not worth doing #7543.

I would have preferred if there were some evidence that these checks are actually been fixed as this PR does not include any changes to the CI

I'm okay to iterate over this

@purnesh42H
Copy link
Contributor

I spoke to @purnesh42H offline and seems like @janardhanvissa is working on all the remaining fixes (expect for unused-parameters) at the moment. Seems like it's not worth doing #7543.

I would have preferred if there were some evidence that these checks are actually been fixed as this PR does not include any changes to the CI

I'm okay to iterate over this

Thanks Arvind. Yeah we can rely on check annotations in the PR /files view. Even though they are not enforced in the PR checks, they do appear there as warnings. So, If they are not appearing anymore in the changed files then we can conclude they are fixed.

@purnesh42H purnesh42H merged commit 3d95421 into grpc:master Aug 23, 2024
13 checks passed
@purnesh42H purnesh42H mentioned this pull request Aug 30, 2024
13 tasks
@janardhanvissa janardhanvissa deleted the godoclinter branch September 16, 2024 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants