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.

Sign up for GitHub

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

Jump to bottom

add additional checks to golangci-lint #5567

Closed
1 task done
rm3l opened this issue Mar 17, 2022 · 0 comments · Fixed by #5687
Closed
1 task done

add additional checks to golangci-lint #5567

rm3l opened this issue Mar 17, 2022 · 0 comments · Fixed by #5687
Assignees
@rm3l
Labels
kind/task Issue is actionable task

Comments

@rm3l
Copy link
Member

rm3l commented Mar 17, 2022 •
edited
Loading

/kind task

configure golangci-lint (https://github.com/redhat-developer/odo/blob/main/.golangci.yaml) to perform two following additional checks:

check 1

This follows a discussion in #5557, related to the removal of the deprecated github.com/pkg/errors package.

To prevent any future use of this deprecated package, I was wondering if we shouldn't have a check that makes sure this package is not introduced again?

I don't know to what extent this is do-able with what we already have at this time, but this could be a check in CI, a Git hook, a custom linter, etc., which would simply grep for such occurrence..

Originally posted by @rm3l in #5557 (comment)

check 2

/kind documentation

What mistake did you find / what is missing in the documentation?

See: https://github.com/golang/go/wiki/CodeReviewComments#error-strings

All error messages should not be capitalize or end with punctuation.

Unless it's the most top level error message.

What is the relevance of it?

A lot of our error output is inconsistent throughout odo

#5611

  • fix all errors, to make check pass
@rm3l rm3l mentioned this issue Mar 17, 2022
Merged
3 tasks
@kadel kadel changed the title Add check for unwanted dependencies add additional checks to golangci-lint Mar 31, 2022
rm3l added a commit to rm3l/odo that referenced this issue Apr 22, 2022
@rm3l
[redhat-developer#5567] Add check preventing the use of the deprecate…
8cee343 
…d 'github.com/pkg/errors' package
rm3l added a commit to rm3l/odo that referenced this issue Apr 22, 2022
@rm3l
[redhat-developer#5567] Upgrade golangci-lint to 1.37.0, so we can us…
53aeb39 
…e 'revive' Linter

- revive is a drop-in replacement for the deprecated golint Linter
- revive will allow to check for error strings

Note: We are purposely not using the latest golangci-lint (1.45.0)
but the minimum version from which revive is available.
This is because the latest 1.45.0 reports additional linting issues
(from govet and staticcheck for example). And fixing those side errors
is outside of the scope of this issue.
Also some of those issues are already fixed in redhat-developer#5497 (update to Go 1.17).
rm3l added a commit to rm3l/odo that referenced this issue Apr 22, 2022
@rm3l
[redhat-developer#5567] Add check preventing the use of the deprecate…
bc25680 
…d 'github.com/pkg/errors' package
rm3l added a commit to rm3l/odo that referenced this issue Apr 22, 2022
@rm3l
[redhat-developer#5567] Upgrade golangci-lint to 1.37.0, so we can us…
e5924a2 
…e 'revive' Linter

- revive is a drop-in replacement for the deprecated golint Linter
- revive will allow to check for error strings

Note: We are purposely not using the latest golangci-lint (1.45.0)
but the minimum version from which revive is available.
This is because the latest 1.45.0 reports additional linting issues
(from govet and staticcheck for example). And fixing those side errors
is outside of the scope of this issue.
Also some of those issues are already fixed in redhat-developer#5497 (update to Go 1.17).
@rm3l rm3l mentioned this issue Apr 22, 2022
Merged
3 tasks
openshift-merge-robot pushed a commit that referenced this issue Apr 25, 2022
@rm3l
Add additional checks to golangci-lint (#5567) (#5687)
06550eb 
* Document golangci-lint configuration options

* Do not limit the number of issues reported per linter

* Add check preventing the use of the deprecated 'github.com/pkg/errors' package

* Upgrade golangci-lint to 1.37.0, so we can use 'revive' Linter

- revive is a drop-in replacement for the deprecated golint Linter
- revive will allow to check for error strings

Note: We are purposely not using the latest golangci-lint (1.45.0)
but the minimum version from which revive is available.
This is because the latest 1.45.0 reports additional linting issues
(from govet and staticcheck for example). And fixing those side errors
is outside of the scope of this issue.
Also some of those issues are already fixed in #5497 (update to Go 1.17).

* Configure revive to check for error strings

More rules can be added later on if needed

* Fix issues reported by revive's error-strings rule

Some rules are purposely ignored when the
error messages represent top-level errors that are
displayed to be displayed as is to end users

* Fix more error-strings issues

For some reason, those were not reported by revive's error-strings rule,
but only by GoLand inspection tool.

* Fix missing `revive:disable:error-strings` comment directive

Also replace "fmt.Errorf" by "errors.New" when the error message is static
@rm3l rm3l mentioned this issue Apr 27, 2022
Merged
3 tasks
cdrage pushed a commit to cdrage/odo that referenced this issue Aug 31, 2022
@rm3l @cdrage
Add additional checks to golangci-lint (redhat-developer#5567) (redha…
95c23f0 
…t-developer#5687)

* Document golangci-lint configuration options

* Do not limit the number of issues reported per linter

* Add check preventing the use of the deprecated 'github.com/pkg/errors' package

* Upgrade golangci-lint to 1.37.0, so we can use 'revive' Linter

- revive is a drop-in replacement for the deprecated golint Linter
- revive will allow to check for error strings

Note: We are purposely not using the latest golangci-lint (1.45.0)
but the minimum version from which revive is available.
This is because the latest 1.45.0 reports additional linting issues
(from govet and staticcheck for example). And fixing those side errors
is outside of the scope of this issue.
Also some of those issues are already fixed in redhat-developer#5497 (update to Go 1.17).

* Configure revive to check for error strings

More rules can be added later on if needed

* Fix issues reported by revive's error-strings rule

Some rules are purposely ignored when the
error messages represent top-level errors that are
displayed to be displayed as is to end users

* Fix more error-strings issues

For some reason, those were not reported by revive's error-strings rule,
but only by GoLand inspection tool.

* Fix missing `revive:disable:error-strings` comment directive

Also replace "fmt.Errorf" by "errors.New" when the error message is static
@rm3l rm3l added kind/task Issue is actionable task v3 labels Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rm3l rm3l

Labels
kind/task Issue is actionable task
Projects
No open projects
odo v3-beta1
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add additional checks to golangci-lint (#5567) rm3l/odo
1 participant
@rm3l