Enable unconvert linter #3171
Merged
Enable unconvert linter #3171
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kubevirt-bot
added
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
kubevirt-bot
removed
the
do-not-merge/work-in-progress
kubevirt-bot
added
dco-signoff: no
|
/lgtm |
This linter's doc describes it as: The unconvert program analyzes Go packages to identify unnecessary type conversions; i.e., expressions T(x) where x already has type T. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>
It is best to show all warnings at once than to reveal them piece-meal, particularly in CI where the feedback loop can be a bit slow. By default, linters may only print the same message three times (https://golangci-lint.run/usage/configuration/#issues-configuration) The unconvert linter always prints the same message, so it specially affected by this setting. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>
Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mhenriks The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubevirt-bot
added
the
approved
7 tasks
akalenyu
pushed a commit
to akalenyu/containerized-data-importer
that referenced
this pull request
May 1, 2024
* Enable unconvert linter This linter's doc describes it as: The unconvert program analyzes Go packages to identify unnecessary type conversions; i.e., expressions T(x) where x already has type T. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Unrestrict the number of linter warnings It is best to show all warnings at once than to reveal them piece-meal, particularly in CI where the feedback loop can be a bit slow. By default, linters may only print the same message three times (https://golangci-lint.run/usage/configuration/#issues-configuration) The unconvert linter always prints the same message, so it specially affected by this setting. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Remove redundant type conversions Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> --------- Signed-off-by: Edu Gómez Escandell <egomez@redhat.com>
kubevirt-bot
added a commit
that referenced
this pull request
May 3, 2024
…intenance (#3227) * Double number of e2e parallel nodes from 3 to 6 (#3156) We have some feedback from different platform testing that this can work tests (not resource) wise. Let's give it a try here as well Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> * Support IPv6 in controller GetMetricsURL (#3165) Joining hostname+port with string concatenation leads to wrong URLs when the hostame is an IPv6: HOST PORT Sprintf CORRECT ::1 8000 ::1:8000 [::1]:8000 To avoid this issue, it's best to use net.JoinHostPort. I added a test that verifies this fix. This type of issue can be discovered running the following command: golangci-lint run ./... --enable nosprintfhostport Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Enable misspell linter and fix spelling errors (#3164) * Add misspell to list of linters Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Fix spelling errors Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> --------- Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Remove openshift dep on api (#3172) Less deps the better! Could be problematic for projects importing CDI and openshift. Signed-off-by: Michael Henriksen <mhenriks@redhat.com> * Enable unconvert linter (#3171) * Enable unconvert linter This linter's doc describes it as: The unconvert program analyzes Go packages to identify unnecessary type conversions; i.e., expressions T(x) where x already has type T. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Unrestrict the number of linter warnings It is best to show all warnings at once than to reveal them piece-meal, particularly in CI where the feedback loop can be a bit slow. By default, linters may only print the same message three times (https://golangci-lint.run/usage/configuration/#issues-configuration) The unconvert linter always prints the same message, so it specially affected by this setting. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Remove redundant type conversions Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> --------- Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Enable linters checking error handling (#3177) * Enable errorlint linter Per the readme: > go-errorlint is a source code linter for Go software that can be used to find code that will cause problems with the error wrapping scheme introduced in Go 1.13. https://github.com/polyfloyd/go-errorlint Essentially, it checks that you properly use errors.Is and errors.As It also forces you to use %w, but that is a bit too much. Using %v is fine when you don't want to leak the internal error types of the package. This is why I disabled this setting. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Replace error comparisons with errors.Is Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Replace error type assertions with errors.As Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Enable errname linter Per the readme: > Checks that sentinel errors are prefixed with the Err and error > types are suffixed with the Error. https://github.com/Antonboom/errname Not a big deal, but helps keep the naming consistent. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Fix errname linter warnings Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> --------- Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Run bazelisk run //robots/cmd/uploader:uploader -- -workspace /home/prow/go/src/github.com/kubevirt/project-infra/../containerized-data-importer/WORKSPACE -dry-run=false (#3199) Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com> * CVE 2024-24786 fix: Bump google.golang.org/protobuf from 1.31.0 to 1.33.0 (#3195) * Upgrade google.golang.org/protobuf This solves CVE-2024-24786 https://www.cve.org/CVERecord?id=CVE-2024-24786 Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Update checksum and vendoring Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> --------- Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Enable durationcheck linter (#3176) * Enable durationcheck linter > Check for two durations multiplied together. https://github.com/charithe/durationcheck Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Fix misuse of time.Duration I also had to rename the variable because go-statickcheck complains about a time.Duration having the suffix 'Sec'. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> --------- Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Enable nakedret linter (#3178) * Enable nakedret linter > Checks that functions with naked returns are not longer than a maximum > size (can be zero). https://github.com/alexkohler/nakedret Relevant from the GO style guide: > Naked returns are okay if the function is a handful of lines. Once > it’s a medium sized function, be explicit with your return values. https://go.dev/wiki/CodeReviewComments#named-result-parameters I think a naked return is never easier to read than a regular return, so I set the linter to always complain about it. Disagreements are welcome. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Refactor functions with naked returns Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> --------- Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Clean up leftover snapshot sources from DataImportCron tests (#3123) * Clean up leftover snapshot sources from DataImportCron tests The dataimportcron tests may affect existing crons in the environment (in addition to the ones deployed by testing) so we are better off cleaning up after ourselves. Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> * Add watch for volumesnapshot delete Although we don't support seamless transition from volumesnapshot->pvc sources (we hope you stay on snapshots if they scale better for your storage) this will still be needed in most cases when someone switches and manually deletes snapshots. Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> --------- Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> * Enable autoformatting linters (#3179) * Enable gofmt linter From the docs: > Gofmt checks whether code was gofmt-ed. By default this tool runs with > -s option to check for code simplification. https://golangci-lint.run/usage/linters/#gofmt Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Run gomft on the project Ran this command after adding the gofmt linter: golangci-lint run ./... --fix Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Enable whitespace linter From the docs: > Whitespace is a linter that checks for unnecessary newlines at the > start and end of functions, if, for, etc. https://golangci-lint.run/usage/linters/#whitespace Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Run whitespace on the project Ran this command after adding the whitespace linter: golangci-lint run ./... --fix Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Enable GCI linter Per the docs: > Gci controls Go package import order and makes it always deterministic. https://golangci-lint.run/usage/linters/#gci NOTE: I noticed that many files separate their imports in a particular way, so I set the linter to enforce this standard. Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Run GCI on the project Ran this command after adding the GCI linter: golangci-lint run ./... --fix Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> --------- Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Enable dupword linter (#3175) * Enable dupword linter Per the readme: > A linter that checks for duplicate words in the source code (usually miswritten) https://github.com/Abirdcfly/dupword https://golangci-lint.run/usage/linters/#dupword Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Remove duplicate words in comments and strings Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> --------- Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> * Change clone strategy and cron sources format for LVMS (#3196) Following some investigation we are now confident this tuning is benefical for LVMS: - CSI clone is effectively implemented the same as snapshotting because both already use lvm2 thinly provisioned snapshots under the hood - Snapshots being COW, it makes total sense to store cron sources in the snapshot format instead of PVC: ``` A snapshot of a thin logical volume also creates a thin logical volume. This consumes no data space until a COW operation is required, or until the snapshot itself is written. ``` Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> * Set default clone strategy & cron format for trident to snapshot (#3209) There's a bug in the trident CSI driver that causes a snapshot to be left over following each CSI clone: NetApp/trident#901 This is becoming a problem rapidly once one hits the snapshot limit per volume (golden image): https://kb.netapp.com/onprem/ontap/os/Maximum_number_of_snapshots_supported_by_ONTAP Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> --------- Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> Signed-off-by: Edu Gómez Escandell <egomez@redhat.com> Signed-off-by: Michael Henriksen <mhenriks@redhat.com> Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com> Co-authored-by: Edu Gómez Escandell <edu1997xyz@gmail.com> Co-authored-by: Edu Gómez Escandell <egomez@redhat.com> Co-authored-by: Michael Henriksen <mhenriks@redhat.com> Co-authored-by: kubevirt-bot <kubevirtbot@redhat.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Enables the
unconvertgolangci-lint linter, and fixes all its warnings.Special notes for your reviewer:
This linter's doc describes it as:
Release note: