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

Cosmetic fixes #7

Merged
merged 3 commits into from
Dec 15, 2023
Merged

Cosmetic fixes #7

merged 3 commits into from
Dec 15, 2023

Conversation

Antonboom
Copy link
Contributor

@Antonboom Antonboom commented Dec 7, 2023

Здравствуйте!

Я видел, что вы меня тегали в прошлом PR, но не было времени добраться.

Этот PR опциональный, можете его деклайнить, если что-то не понравится.
Постараюсь расставить комментарии со объяснением.

Также, что меня смутило:

  1. Неконсистентность между package factory (too common) и go-factory-lint. Мб сделать factorylint? Или пакет сократить до go-factory? Или глянуть, что там у соседей
gocognit
goconst
gocritic
gocyclo
godot
godox
gofmt
gofumpt
goheader
goimports
golint
gomnd
gomoddirectives
gomodguard
gosimple
gosec
gosmopolitan
govet
  1. По-английски будет не onlyPackageGlobs, а packageGlobsOnly, хз насколько критично.

  2. Зависимость на сторонний пакет glob (правда golangci-lint от него уже зависит). Есть ощущение, что от всего глоба там нужно только многоточие (по сути pkg prefix). Но использовать по минимуму сторонние пакеты в общедоступном модуле – это мои предрассудки, продиктованные набитыми шишками. Также можно вместо stringsFlag сделать сразу globsFlag, тогда он будет бахать на момент парсинга флага, а не ругаться N раз, где N – число анализируемых пакетов (Pass).


golangci/golangci-lint#4196

README.md Show resolved Hide resolved
factory.go Show resolved Hide resolved
factory.go Show resolved Hide resolved
factory.go Show resolved Hide resolved
factory.go Show resolved Hide resolved
factory.go Show resolved Hide resolved
factory.go Outdated Show resolved Hide resolved
lint_test.go Show resolved Hide resolved
@maranqz
Copy link
Owner

maranqz commented Dec 11, 2023

Огромное спасибо, что нашли время и посмотрели.

Также, что меня смутило:

Поддерживаю, поправлю в ближайшую пару недель в этом же PR.

2. stringsFlag replaced with globsFlag
3. onlyPackageGlobs renamed to packageGlobsOnly
@maranqz maranqz merged commit ab0ddc6 into maranqz:main Dec 15, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants