.github/workflows/ci.yaml: update golangci-lint to v1.33.0 #1151
Conversation
|
I suppose we need to wait for new release or Docker image update for CI to pass on this. See golangci/golangci-lint#1483 for more details. |
invidian
force-pushed
the
invidian/update-linter
branch
from
6dbc987 to
5bd2714
Compare
invidian
changed the title
invidian
force-pushed
the
invidian/update-linter
branch
from
5bd2714 to
85af08c
Compare
|
I just want to confirm, was the skip option not working before? |
Hmm, I don't remember now, but I tested it 😞 According to the action logs, skipped names are passed simply as a parameters, so resulting options were like |
invidian
force-pushed
the
invidian/update-linter
branch
from
85af08c to
d7d59d8
Compare
.github/workflows/ci.yaml
Outdated
| @@ -49,7 +49,7 @@ jobs: | |||
| - name: Codespell test | |||
| uses: codespell-project/actions-codespell@master | |||
| with: | |||
| skip: vendor, ./lokoctl, *.png, assets/charts/components/*, docs/images/*, | |||
| skip: ./vendor,./lokoctl,*.png,./assets/charts/components/*,./docs/images/*,./assets/charts/components/cert-manager,./assets/charts/components/rook,./assets/charts/components/prometheus-operator,./assets/charts/components/velero,./assets/charts/components/cluster-autoscaler,./assets/charts/components/contour/crds,./assets/charts/components/openebs-operator/README.md,./assets/charts/control-plane/calico/crds,./docs/images/lokomotive-example.gif | |||
There was a problem hiding this comment.
Doesn't it skip the entire tree? Like we have assets/charts/components/* and then specific paths again?
There was a problem hiding this comment.
I was also confused, why assets/charts/components/* didn't work
There was a problem hiding this comment.
Ah true, I think ./assets/charts/components/* should be removed, so we are aware of new typos and we have a chance to fix them upstream.
There was a problem hiding this comment.
But why check them in some and ignore them in others? How is this decided?
There was a problem hiding this comment.
We ignore projects which are already in the codebase and contain typos. If you let's say update chart X and update brings typos, the correct thing to do is to send patch upstream fixing them and adding it to the list here until it has been fixed upstream, so it does not block merging on our side.
There was a problem hiding this comment.
I think we are just creating more work by doing that. We should simply ignore the entire assets directory. We take care of things that are in our control.
The problems from upstream should not add to our mental burden.
There was a problem hiding this comment.
I disagree. Given that we actively consume those assets, I believe we should at least contribute typo fixes back.
There was a problem hiding this comment.
I don't agree on on the specific skipping we should rather skip the whole assets dir like I mentioned before, because I can see that we are creating more work for ourselves. I don't mean we should not fix the upstream for spellings but should not be at the expense of Lokomotive development resources.
Since I disagree with the change, please go ahead with merging if it is the right thing to do.
There was a problem hiding this comment.
@iaguis what do you think?
invidian
force-pushed
the
invidian/update-linter
branch
3 times, most recently
from
4f1c135 to
48210f5
Compare
|
@invidian can you please update it to https://github.com/golangci/golangci-lint/releases/tag/v1.33.0? |
invidian
force-pushed
the
invidian/update-linter
branch
from
48210f5 to
d2c7eed
Compare
Done, though I didn't test this version yet. And I already found some issues with it: |
invidian
changed the title
I think we can merge it. I run it locally and didn't run into any issues so far. |
Signed-off-by: Mateusz Gozdek <[email protected]>
invidian
force-pushed
the
invidian/update-linter
branch
from
2544dca to
83f6c43
Compare
Using spaces in it make it only check listed files I think. Also sync with .codespell.skip file, so we get consistent behavior across local execution and CI. Also avoid using wildcard on components, so we have a chance to fix reported typos upstream before we skip them from checking. Signed-off-by: Mateusz Gozdek <[email protected]>
Signed-off-by: Mateusz Gozdek <[email protected]>
invidian
force-pushed
the
invidian/update-linter
branch
from
83f6c43 to
c22ba6c
Compare
No description provided.