TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 104 Pull requests 7 Projects Releases 2 Activity Actions

Enable unparam and unconvert linters #1598

Merged
dstepanov-yadro merged 3 commits from dstepanov-yadro/frostfs-node:feat/add_linters into master 2025-01-14 08:27:24 +00:00
Conversation 1 Commits 3 Files changed 42 +166 -220
dstepanov-yadro commented 2025-01-13 14:45:46 +00:00
Member
Copy link

Add unparam and unconvert linters to make code cleaner.

Add `unparam` and `unconvert` linters to make code cleaner.
👍 1
dstepanov-yadro added 2 commits 2025-01-13 14:45:46 +00:00 
To drop unnecessary conversions.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
[#9999] node: Enable unparam linter
All checks were successful
DCO action / DCO (pull_request) Successful in 29s
Details
Vulncheck / Vulncheck (pull_request) Successful in 1m7s
Details
Pre-commit hooks / Pre-commit (pull_request) Successful in 1m31s
Details
Build / Build Components (pull_request) Successful in 1m37s
Details
Tests and linters / Run gofumpt (pull_request) Successful in 2m7s
Details
Tests and linters / Staticcheck (pull_request) Successful in 2m11s
Details
Tests and linters / Lint (pull_request) Successful in 2m50s
Details
Tests and linters / Tests (pull_request) Successful in 3m41s
Details
Tests and linters / gopls check (pull_request) Successful in 4m6s
Details
Tests and linters / Tests with -race (pull_request) Successful in 4m16s
Details
a4481ddb4d 
To drop unnecessary parameters and return values.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
dstepanov-yadro requested review from storage-core-committers 2025-01-13 14:45:47 +00:00
dstepanov-yadro requested review from storage-core-developers 2025-01-13 14:45:47 +00:00
dstepanov-yadro force-pushed feat/add_linters from a4481ddb4d to 6d18cbd791 2025-01-13 14:50:53 +00:00 Compare
a-savchuk approved these changes 2025-01-13 15:31:33 +00:00
Dismissed
a-savchuk left a comment
Member
Copy link

unparam looks awesome🔥

node: Enable unconvert linters

Did you consider another scope for your commit message? For example, golangci or ci

Here are some of your commits from the past

[#1388] golangci: Make `unused` linter stricker
    
    Add aditional checks. The most important false positive - structs used as
    map keys.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>

...

[#857] golangci: Add protogetter linter
    
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
`unparam` looks awesome🔥 > node: Enable unconvert linters Did you consider another scope for your commit message? For example, `golangci` or `ci` Here are some of your commits from the past ``` [#1388] golangci: Make `unused` linter stricker Add aditional checks. The most important false positive - structs used as map keys. Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com> ... [#857] golangci: Add protogetter linter Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com> ```
dstepanov-yadro force-pushed feat/add_linters from 6d18cbd791 to fb928616cc 2025-01-14 06:07:06 +00:00 Compare
dstepanov-yadro commented 2025-01-14 06:07:15 +00:00
Author
Member
Copy link

unparam looks awesome🔥

node: Enable unconvert linters

Did you consider another scope for your commit message? For example, golangci or ci

Here are some of your commits from the past

[#1388] golangci: Make `unused` linter stricker
    
    Add aditional checks. The most important false positive - structs used as
    map keys.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>

...

[#857] golangci: Add protogetter linter
    
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>

Done

> `unparam` looks awesome🔥 > > > > node: Enable unconvert linters > > Did you consider another scope for your commit message? For example, `golangci` or `ci` > > Here are some of your commits from the past > ``` > [#1388] golangci: Make `unused` linter stricker > > Add aditional checks. The most important false positive - structs used as > map keys. > > Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com> > > ... > > [#857] golangci: Add protogetter linter > > Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com> > ``` Done
👍 1
fyrchik approved these changes 2025-01-14 06:56:51 +00:00
Dismissed
pkg/local_object_storage/engine/inhume.go Outdated
@ -30,6 +30,8 @@ type InhumePrm struct {
// InhumeRes encapsulates results of inhume operation.
type InhumeRes struct{}
var inhumeRes = InhumeRes{}
fyrchik commented 2025-01-14 06:56:01 +00:00
Owner
Copy link

Why was this a problem?

Why was this a problem?
dstepanov-yadro commented 2025-01-14 07:08:27 +00:00
Author
Member
Copy link

Linter says that all return values of InhumeRes are the same, so this looks like there is no return value requered.
But we have an unspoken agreement to return the result and the error.
To make everyone happy, a global variable was introduced.

Linter says that all return values of `InhumeRes` are the same, so this looks like there is no return value requered. But we have an unspoken agreement to return the result and the error. To make everyone happy, a global variable was introduced.
fyrchik commented 2025-01-14 07:29:14 +00:00
Owner
Copy link

I thought this was the reason we wanted this linter: what about removing Res struct where it is not needed?
This global variable looks like lint:ignore and may introduce problems later (were we to add a field and modify it somehow).

I thought this was the reason we wanted this linter: what about removing `Res` struct where it is not needed? This global variable looks like `lint:ignore` and may introduce problems later (were we to add a field and modify it somehow).
dstepanov-yadro commented 2025-01-14 07:41:08 +00:00
Author
Member
Copy link

You're trying to destroy long-standing foundations!

You're trying to destroy long-standing foundations!
fyrchik commented 2025-01-14 07:50:50 +00:00
Owner
Copy link

I am trying to refine them.
The addition of this linter is a worthy reason.

I am trying to refine them. The addition of this linter is a worthy reason.
dstepanov-yadro commented 2025-01-14 08:19:08 +00:00
Author
Member
Copy link

Ok, fixed

Ok, fixed
acid-ant approved these changes 2025-01-14 08:18:28 +00:00
Dismissed
dstepanov-yadro added 1 commit 2025-01-14 08:19:02 +00:00
[#1598] engine: Drop unnecessary result structs
All checks were successful
DCO action / DCO (pull_request) Successful in 31s
Details
Tests and linters / Run gofumpt (pull_request) Successful in 29s
Details
Vulncheck / Vulncheck (pull_request) Successful in 54s
Details
Build / Build Components (pull_request) Successful in 1m29s
Details
Pre-commit hooks / Pre-commit (pull_request) Successful in 1m28s
Details
Tests and linters / gopls check (pull_request) Successful in 2m15s
Details
Tests and linters / Lint (pull_request) Successful in 2m41s
Details
Tests and linters / Staticcheck (pull_request) Successful in 2m49s
Details
Tests and linters / Tests (pull_request) Successful in 3m52s
Details
Tests and linters / Tests with -race (pull_request) Successful in 5m1s
Details
Vulncheck / Vulncheck (push) Successful in 1m5s
Details
Pre-commit hooks / Pre-commit (push) Successful in 1m19s
Details
Build / Build Components (push) Successful in 1m36s
Details
Tests and linters / gopls check (push) Successful in 2m16s
Details
Tests and linters / Run gofumpt (push) Successful in 2m32s
Details
Tests and linters / Staticcheck (push) Successful in 2m49s
Details
Tests and linters / Lint (push) Successful in 3m10s
Details
Tests and linters / Tests (push) Successful in 3m12s
Details
Tests and linters / Tests with -race (push) Successful in 4m15s
Details
eff95bd632 
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
dstepanov-yadro dismissed a-savchuk's review 2025-01-14 08:19:02 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

dstepanov-yadro dismissed fyrchik's review 2025-01-14 08:19:02 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

dstepanov-yadro dismissed acid-ant's review 2025-01-14 08:19:02 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

acid-ant approved these changes 2025-01-14 08:21:17 +00:00
aarifullin approved these changes 2025-01-14 08:21:36 +00:00
fyrchik approved these changes 2025-01-14 08:26:54 +00:00
dstepanov-yadro merged commit eff95bd632 into master 2025-01-14 08:27:24 +00:00
dstepanov-yadro deleted branch feat/add_linters 2025-01-14 08:27:25 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
acid-ant
aarifullin
fyrchik
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
badger

Badger DB related issues

  
frostfs-adm

   
frostfs-cli

   
frostfs-ir

   
frostfs-lens

   
frostfs-node

   
good first issue

Good for newcomers

  
triage
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
5 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#1598
No description provided.
Delete branch "dstepanov-yadro/frostfs-node:feat/add_linters"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?