TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 100 Pull requests 4 Projects Releases 1 Activity Actions

refactoring: make unused linter stricker #1388

Merged
dstepanov-yadro merged 15 commits from dstepanov-yadro/frostfs-node:refactoring/drop_unused into master 2024-09-25 08:55:39 +00:00
Conversation 2 Commits 15 Files changed 28 +32 -124
dstepanov-yadro commented 2024-09-23 06:22:11 +00:00
Member
Copy link

Enable some unused additional parameters: https://golangci-lint.run/usage/linters/#unused
The most important false positive case is structs used as map keys, so they are marked with nolint annotations.

Enable some unused additional parameters: https://golangci-lint.run/usage/linters/#unused The most important false positive case is structs used as map keys, so they are marked with `nolint` annotations.
dstepanov-yadro added 15 commits 2024-09-23 06:22:12 +00:00 
Add aditional checks. The most important false positive - structs used as
map keys.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Although these fields could be deleted, I annotated them so that all the
metrics used would be defined in one place.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
Used as map key.

Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
[#9999] node: Drop unused
All checks were successful
DCO action / DCO (pull_request) Successful in 51s
Details
Tests and linters / Run gofumpt (pull_request) Successful in 1m10s
Details
Pre-commit hooks / Pre-commit (pull_request) Successful in 2m11s
Details
Vulncheck / Vulncheck (pull_request) Successful in 2m4s
Details
Build / Build Components (pull_request) Successful in 2m17s
Details
Tests and linters / gopls check (pull_request) Successful in 2m33s
Details
Tests and linters / Staticcheck (pull_request) Successful in 2m59s
Details
Tests and linters / Lint (pull_request) Successful in 3m24s
Details
Tests and linters / Tests (pull_request) Successful in 6m50s
Details
Tests and linters / Tests with -race (pull_request) Successful in 6m55s
Details
054996c217 
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
dstepanov-yadro force-pushed refactoring/drop_unused from 054996c217 to 78b0278746 2024-09-23 06:24:39 +00:00 Compare
dstepanov-yadro reviewed 2024-09-23 06:25:33 +00:00
cmd/frostfs-lens/internal/schema/common/raw.go Outdated
@ -8,3 +8,3 @@
type RawEntry struct {
key, value []byte
key []byte
dstepanov-yadro commented 2024-09-23 06:25:32 +00:00
Author
Member
Copy link

@a-savchuk please review

@a-savchuk please review
dstepanov-yadro reviewed 2024-09-23 06:26:34 +00:00
pkg/services/object/put/prm.go Outdated
@ -20,4 +16,2 @@
traverseOpts []placement.Option
relay func(context.Context, client.NodeInfo, client.MultiAddressClient) error
dstepanov-yadro commented 2024-09-23 06:26:34 +00:00
Author
Member
Copy link

@aarifullin please review. Looks like this could be deleted after Patch related refactorings

@aarifullin please review. Looks like this could be deleted after `Patch` related refactorings
dstepanov-yadro requested review from storage-core-committers 2024-09-23 06:26:45 +00:00
dstepanov-yadro requested review from storage-core-developers 2024-09-23 06:26:49 +00:00
dstepanov-yadro requested review from aarifullin 2024-09-23 06:27:05 +00:00
dstepanov-yadro requested review from a-savchuk 2024-09-23 06:27:07 +00:00
acid-ant commented 2024-09-23 06:37:31 +00:00
Member
Copy link

Is it possible to suppress check for cmode for ir?

Is it possible to suppress check for `cmode` for `ir`?
dstepanov-yadro force-pushed refactoring/drop_unused from 78b0278746 to 6da36ff7fc 2024-09-23 07:51:50 +00:00 Compare
dstepanov-yadro commented 2024-09-23 07:58:36 +00:00
Author
Member
Copy link

Is it possible to suppress check for cmode for ir?

Ok

> Is it possible to suppress check for `cmode` for `ir`? Ok
dstepanov-yadro force-pushed refactoring/drop_unused from 6da36ff7fc to ba3d1c5a41 2024-09-23 08:09:47 +00:00 Compare
aarifullin reviewed 2024-09-24 08:39:24 +00:00
pkg/services/object/put/prm.go Outdated
@ -20,4 +16,2 @@
traverseOpts []placement.Option
relay func(context.Context, client.NodeInfo, client.MultiAddressClient) error
aarifullin commented 2024-09-24 08:39:24 +00:00
Member
Copy link

Oh, no. My refactoring was incorrect. Please, do not apply this change until my fix is merged

Oh, no. My refactoring was incorrect. Please, do not apply this change until my fix is merged
aarifullin marked this conversation as resolved
dstepanov-yadro force-pushed refactoring/drop_unused from ba3d1c5a41 to b5a29c3e34 2024-09-24 09:11:09 +00:00 Compare
dstepanov-yadro added 1 commit 2024-09-24 09:13:37 +00:00
[#1388] putSvc: Drop unused
All checks were successful
Tests and linters / Run gofumpt (pull_request) Successful in 1m21s
Details
DCO action / DCO (pull_request) Successful in 1m33s
Details
Vulncheck / Vulncheck (pull_request) Successful in 2m31s
Details
Pre-commit hooks / Pre-commit (pull_request) Successful in 2m41s
Details
Build / Build Components (pull_request) Successful in 2m54s
Details
Tests and linters / Staticcheck (pull_request) Successful in 2m52s
Details
Tests and linters / gopls check (pull_request) Successful in 3m6s
Details
Tests and linters / Lint (pull_request) Successful in 3m37s
Details
Tests and linters / Tests (pull_request) Successful in 4m54s
Details
Tests and linters / Tests with -race (pull_request) Successful in 6m14s
Details
7a4f1fe9ee 
Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
aarifullin approved these changes 2024-09-24 09:28:47 +00:00
Dismissed
acid-ant approved these changes 2024-09-24 10:22:51 +00:00
Dismissed
achuprov approved these changes 2024-09-24 11:18:31 +00:00
Dismissed
dstepanov-yadro force-pushed refactoring/drop_unused from 7a4f1fe9ee to aa763c6bcd 2024-09-24 12:15:25 +00:00 Compare
a-savchuk requested changes 2024-09-24 12:55:08 +00:00
Dismissed
cmd/frostfs-lens/internal/schema/common/raw.go Outdated
@ -7,13 +7,13 @@ import (
)
type RawEntry struct {
key, value []byte
a-savchuk commented 2024-09-24 12:52:55 +00:00
Member
Copy link

For a raw entry, as for any entry, we wanna be able to see its dump (see DetailedString method). Both fields key and value are intended to be shown in a dump view. Could we ignore the lint warning here? or should we do something trickier like this

func (r *RawEntry) DetailedString() string {
	_ = r.value // both value and key need to be used in dump 
	return spew.Sdump(r)
}

and add an additional comment to describe that decision?

For a raw entry, as for any entry, we wanna be able to see its dump (see `DetailedString` method). Both fields `key` and `value` are intended to be shown in a dump view. Could we ignore the lint warning here? or should we do something trickier like this ```go func (r *RawEntry) DetailedString() string { _ = r.value // both value and key need to be used in dump return spew.Sdump(r) } ``` and add an additional comment to describe that decision?
dstepanov-yadro commented 2024-09-24 13:00:59 +00:00
Author
Member
Copy link

fixed

fixed
a-savchuk marked this conversation as resolved
cmd/frostfs-lens/internal/schema/writecache/parsers.go Outdated
@ -57,7 +57,5 @@ func DefaultRecordParser(key, value []byte) (common.SchemaEntry, common.Parser,
r.addr.SetContainer(cnr)
r.addr.SetObject(obj)
r.data = value[:]
a-savchuk commented 2024-09-24 12:54:52 +00:00
Member
Copy link

Same comment as the one for RawEntry. We should keep that field for DetailedString method

Same comment as the one for `RawEntry`. We should keep that field for `DetailedString` method
dstepanov-yadro commented 2024-09-24 13:01:09 +00:00
Author
Member
Copy link

fixed

fixed
a-savchuk marked this conversation as resolved
cmd/frostfs-lens/internal/schema/writecache/types.go Outdated
@ -16,7 +16,6 @@ type (
DefaultRecord struct {
addr oid.Address
data []byte
a-savchuk commented 2024-09-24 12:54:33 +00:00
Member
Copy link

Same comment as the one for RawEntry. We should keep that field for DetailedString method

Same comment as the one for `RawEntry`. We should keep that field for `DetailedString` method
dstepanov-yadro commented 2024-09-24 13:01:13 +00:00
Author
Member
Copy link

fixed

fixed
a-savchuk marked this conversation as resolved
dstepanov-yadro force-pushed refactoring/drop_unused from aa763c6bcd to 62c8e6554c 2024-09-24 13:00:07 +00:00 Compare
dstepanov-yadro dismissed aarifullin's review 2024-09-24 13:00:08 +00:00
Reason:

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

dstepanov-yadro dismissed acid-ant's review 2024-09-24 13:00:08 +00:00
Reason:

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

dstepanov-yadro dismissed achuprov's review 2024-09-24 13:00:08 +00:00
Reason:

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

a-savchuk approved these changes 2024-09-24 13:04:16 +00:00
dstepanov-yadro requested review from acid-ant 2024-09-24 13:12:19 +00:00
dstepanov-yadro requested review from aarifullin 2024-09-24 13:12:19 +00:00
dstepanov-yadro requested review from achuprov 2024-09-24 13:12:20 +00:00
achuprov approved these changes 2024-09-24 13:45:00 +00:00
acid-ant approved these changes 2024-09-24 14:05:31 +00:00
aarifullin approved these changes 2024-09-25 08:21:10 +00:00
dstepanov-yadro merged commit 772b471aab into master 2024-09-25 08:55:39 +00:00
dstepanov-yadro deleted branch refactoring/drop_unused 2024-09-25 08:55:41 +00:00
dstepanov-yadro referenced this pull request from a commit 2024-09-25 08:55:41 +00:00
[#1388] getSvc: Drop unused
dstepanov-yadro referenced this pull request from a commit 2024-09-25 08:55:41 +00:00
[#1388] engine: Drop unused
dstepanov-yadro referenced this pull request from a commit 2024-09-25 08:55:41 +00:00
[#1388] blobstor: Drop unused
dstepanov-yadro referenced this pull request from a commit 2024-09-25 08:55:41 +00:00
[#1388] metabase: Drop unused
dstepanov-yadro referenced this pull request from a commit 2024-09-25 08:55:42 +00:00
[#1388] signSvc: Drop unused
dstepanov-yadro referenced this pull request from a commit 2024-09-25 08:55:42 +00:00
[#1388] node: Drop unused
dstepanov-yadro referenced this pull request from a commit 2024-09-25 08:55:42 +00:00
[#1388] putSvc: Drop unused
Sign in to join this conversation.
Reviewers
No reviewers
a-savchuk
achuprov
acid-ant
aarifullin
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#1388
No description provided.
Delete branch "dstepanov-yadro/frostfs-node:refactoring/drop_unused"

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?