components-contrib icon indicating copy to clipboard operation
components-contrib copied to clipboard

use docker for linting for github action compatibility

Open mindovermiles262 opened this issue 3 years ago • 5 comments

Signed-off-by: Andrew [email protected]

Description

Uses docker, if available, to run the golangci-lint er which is in sync with version of linting on Github

Issue reference

#1859

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • [x] Code compiles correctly
  • [x] Created/updated tests
  • [x] Extended the documentation

mindovermiles262 avatar Jul 07 '22 15:07 mindovermiles262

User [~/code/go/components-contrib] (ft/docker-lint)
$ which docker
/usr/bin/docker

User [~/code/go/components-contrib] (ft/docker-lint)
$ make lint
# Due to https://github.com/golangci/golangci-lint/issues/580, we need to add --fix for windows
docker run --rm -v /home/dipsea/code/go/components-contrib:/app -w /app golangci/golangci-lint:v1.45.2 golangci-lint run --timeout=20m
[...]

User [~/code/go/components-contrib] (ft/docker-lint)
$ sudo mv /usr/bin/docker /usr/bin/dokr

User [~/code/go/components-contrib] (ft/docker-lint)
$ make lint
# Due to https://github.com/golangci/golangci-lint/issues/580, we need to add --fix for windows
golangci-lint run --timeout=20m
[...]

mindovermiles262 avatar Jul 07 '22 15:07 mindovermiles262

I do not have a windows device to test the where golangci-lint.exe command or the installation/lint/grepping. Can anyone test that for me?

Otherwise, here are the logs from my local testing. Started first without the linter installed, then used the "correct" version, then a different version from the Github CI

$ which golangci-lint

$ make lint
[*] golangci_lint not installed
[*] Installing now...
golangci/golangci-lint info checking GitHub for tag 'v1.45.2'
golangci/golangci-lint info found version: 1.45.2 for v1.45.2/linux/amd64
golangci/golangci-lint info installed /home/dipsea/code/go/bin/golangci-lint
# Due to https://github.com/golangci/golangci-lint/issues/580, we need to add --fix for windows
golangci-lint run --timeout=20m
[ ...SNIP... Lint running with v1.45.2 ]




$ golangci-lint --version
golangci-lint has version 1.45.2 built from 8bdc4d3f on 2022-03-24T11:51:26Z

$ make lint
# Due to https://github.com/golangci/golangci-lint/issues/580, we need to add --fix for windows
golangci-lint run --timeout=20m
[ ...SNIP... Lint running with v1.45.2 ]




$ golangci-lint --version
golangci-lint has version 1.46.2 built from a3336890 on 2022-05-17T11:31:29Z

$ make lint
[!] Your locally installed version of golangci-lint is different from the pipeline
[!] This will likely cause linting issues for you locally
[!] Yours: v1.46.2 Theirs: v1.45.2
# Due to https://github.com/golangci/golangci-lint/issues/580, we need to add --fix for windows
golangci-lint run --timeout=20m
[ ...SNIP... Lint with Errors from using v1.46.2 ]

mindovermiles262 avatar Jul 10 '22 18:07 mindovermiles262

Haven't gotten a chance to look at this yet, but still on my ToDo list

mindovermiles262 avatar Jul 19 '22 18:07 mindovermiles262

For the time being I will convert this PR to a draft. Once the changes have been made feel free to convert it back to a regular PR and we can get this merged.

berndverst avatar Aug 08 '22 17:08 berndverst

Codecov Report

Merging #1861 (a21456d) into master (8b48210) will increase coverage by 0.02%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1861      +/-   ##
==========================================
+ Coverage   37.63%   37.66%   +0.02%     
==========================================
  Files         192      192              
  Lines       23982    23982              
==========================================
+ Hits         9026     9032       +6     
+ Misses      14191    14183       -8     
- Partials      765      767       +2     
Impacted Files Coverage Δ
nameresolution/mdns/mdns.go 71.74% <0.00%> (-0.52%) :arrow_down:
state/in-memory/in_memory.go 46.00% <0.00%> (+3.42%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 08 '22 17:08 codecov[bot]

I think this is ready for review:

Linter Not Installed

--> Throws Error

% make lint
[!] golangci_lint not installed
[!] You can install it from https://golangci-lint.run/usage/install/
[!]   or by running
[!]   curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b /bin
make: *** [verify-linter-installed] Error 1

Linter Version Mismatch

--> Throws Warning, continues with lint

% make lint
[!] Your locally installed version of golangci-lint is different from the pipeline
[!] This will likely cause linting issues for you locally
[!] Yours:  v1.46.2
[!] Theirs: v1.48.0
# Due to https://github.com/golangci/golangci-lint/issues/580, we need to add --fix for windows
golangci-lint run --timeout=20m

mindovermiles262 avatar Aug 18 '22 21:08 mindovermiles262

cc @berndverst

yaron2 avatar Aug 18 '22 22:08 yaron2

I saw @yaron2. I got lots of email notifications for every changed made to this 😄

berndverst avatar Aug 18 '22 22:08 berndverst

@mindovermiles262 please make sure to sign off on your commits -- see the failing DCO check.

berndverst avatar Aug 18 '22 22:08 berndverst

Something is not quite right now :)

Please rebase the branch against contrib/master.

berndverst avatar Aug 18 '22 22:08 berndverst

This PR should not contain all the commits that it does. It should only contain your commits.

Can you please do:

git fetch origin
git rebase origin/master
git push YOURREMOTE --force

berndverst avatar Aug 18 '22 22:08 berndverst

I think I got the sign off's fixed. Let me know if I didn't..

mindovermiles262 avatar Aug 18 '22 22:08 mindovermiles262