search

etcd-io / etcd

Distributed reliable key-value store for the most critical data of a distributed system
https://etcd.io
Apache License 2.0
46.79k stars 9.64k forks source link

[3.4] Linters fail locally, but pass on CI #17472

Open ivanvc opened 4 months ago

ivanvc commented 4 months ago

Bug report criteria

What happened?

There are some issues regarding the state of the linters in the CI.

I don't know the value of markdown_marker. And the bom check is skipped locally, too, with a comment that it can't run with Go modules.

This issue can be split into:

  1. Address the shellcheck violations
  2. Address the revive violations
  3. Ensure the CI/GitHub workflow has shellcheck and revive installed so the checks are not skipped.

What did you expect to happen?

The CI should fail as linter checks are failing.

How can we reproduce it (as minimally and precisely as possible)?

  1. Have locally installed shellcheck and revive, then run PASSES=fmt ./test
  2. Check that the CI skips the checks by going to any build, i.e., expand step "Run set -euo pipefail" from the latest build: https://github.com/etcd-io/etcd/actions/runs/7978454119/job/21783602709#check-step-6

Anything else we need to know?

After fixing the linting issues from 3.5 (#17401 and #17400), I discovered that 3.4 had its own linter issues.

Etcd version (please run commands below)

```console $ etcd --version # paste output here $ etcdctl version # paste output here ```

Etcd configuration (command line flags or environment variables)

# paste your configuration here

Etcd debug information (please run commands below, feel free to obfuscate the IP address or FQDN in the output)

```console $ etcdctl member list -w table # paste output here $ etcdctl --endpoints= endpoint status -w table # paste output here ```

Relevant log output

No response

ivanvc commented 4 months ago

/label area/testing

k8s-ci-robot commented 4 months ago

@ivanvc: The label(s) /label area/testing cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to [this](https://github.com/etcd-io/etcd/issues/17472#issuecomment-1957540123): >/label area/testing Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
henrybear327 commented 2 months ago

Hey @ivanvc, I would like to take this issue!

ivanvc commented 2 months ago

/assign @henrybear327

ivanvc commented 2 months ago

Please take a look at #17400 and #17401 as a reference.

thedtripp commented 1 week ago

Hi @ivanvc @jmhbnz , I'd like to take this issue.

henrybear327 commented 1 week ago

/assign @thedtripp

ahrtr commented 3 days ago

Thanks @ivanvc for raising this issue.

  • Linters shellcheck (silently), markdown_marker, revive, and goword are skipped by the CI (exclamation marks were intentionally added for emphasis)

I think we should make sure the linters aren't skipped, and resolve them one by one in PRs.