Refactoring: resolve frostfs-node innerring linters #185

dstepanov-yadro · 2023-03-29T08:48:53Z

dstepanov-yadro commented

2023-03-29 08:48:53 +00:00

No description provided.

dstepanov-yadro force-pushed refactoring/object-3610-innerring from ac24167034 to 8a518bf7c6

2023-03-29 08:49:49 +00:00

Compare

dstepanov-yadro force-pushed refactoring/object-3610-innerring from 8a518bf7c6 to b3d9ebcc35

2023-03-29 08:53:42 +00:00

Compare

dstepanov-yadro force-pushed refactoring/object-3610-innerring from 0c3b479021 to ab71da0b96

2023-03-29 14:00:54 +00:00

Compare

dstepanov-yadro force-pushed refactoring/object-3610-innerring from daf312c511 to 7e103f33e5

2023-03-29 14:51:06 +00:00

Compare

dstepanov-yadro changed title from ~~WIP: Refactoring: resolve frostfs-node innerring linters~~ to Refactoring: resolve frostfs-node innerring linters

2023-03-30 08:08:42 +00:00

dstepanov-yadro requested review from storage-core-committers 2023-03-30 08:08:51 +00:00

dstepanov-yadro requested review from storage-core-developers 2023-03-30 08:08:51 +00:00

fyrchik reviewed 2023-03-30 11:19:46 +00:00

pkg/innerring/alphabet.go

					
				@ -52,96 +52,56 @@ const (

					lastLetterNum

				)

				var glagolicLetterToString = map[GlagoliticLetter]string{

fyrchik commented

2023-03-30 11:12:51 +00:00

Have you considered using https://pkg.go.dev/golang.org/x/tools/cmd/stringer ?

fyrchik commented

2023-03-30 11:20:15 +00:00

It supports custom names with // az comment, if we need them.

It supports custom names with `// az` comment, if we need them.

dstepanov-yadro commented

2023-03-30 12:06:40 +00:00

No, i haven't.
This tool doesn't support default values as is see

No, i haven't. This tool doesn't support default values as is see

fyrchik marked this conversation as resolved

pkg/innerring/alphabet.go

					
				@ -55,0 +88,4 @@

					yat:             "yat",

					jo:              "jo",

					yu:              "yu",

					smallYus:        "small.yus",

fyrchik commented

2023-03-30 11:11:57 +00:00

BTW, we can have a problem with NNS on a big cluster, must not forget.

😕 1

pkg/innerring/innerring.go Outdated

					
				@ -172,3 +142,1 @@

						if err := starter(); err != nil {

							return err

						}

					err = s.startStarters()

fyrchik commented

2023-03-30 11:15:31 +00:00

Nice name!

😆 1

fyrchik commented

2023-03-30 11:18:11 +00:00

What about this?

func startWorkers(func(chan error), chan error) error {}
...
startWorkers(s.starters, nil)
startWorkers(s.runners, errCh)

Could require more refactoring, though.

What about this? ``` func startWorkers(func(chan error), chan error) error {} ... startWorkers(s.starters, nil) startWorkers(s.runners, errCh) ``` Could require more refactoring, though.

dstepanov-yadro commented

2023-03-30 12:12:21 +00:00

IMHO it's overengineered and overgeneralized. just renamed function.

👍 1

fyrchik marked this conversation as resolved

dstepanov-yadro force-pushed refactoring/object-3610-innerring from 205a663681 to d161f83613

2023-03-30 12:11:20 +00:00

Compare

carpawell approved these changes 2023-03-30 16:07:25 +00:00

fyrchik approved these changes 2023-03-30 19:10:27 +00:00

fyrchik merged commit 8426d25f4b into master

2023-03-30 19:10:35 +00:00