add golangci lint GH action #2976

Glyphack · 2020-10-02T10:04:06Z

use golangci-lint action to lint new code changes on PRs.

Signed-off-by: Shayegan Hooshyari [email protected]

codecov · 2020-10-02T10:08:11Z

Codecov Report

Merging #2976 into main will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2976      +/-   ##
==========================================
+ Coverage   73.35%   73.38%   +0.03%     
==========================================
  Files          95       95              
  Lines        6046     6046              
==========================================
+ Hits         4435     4437       +2     
+ Misses       1510     1509       -1     
+ Partials      101      100       -1

Impacted Files	Coverage Δ
internal/dag/cache.go	`96.13% <0.00%> (+0.77%)`	⬆️

use https://github.com/marketplace/actions/run-golangci-lint to lint code for new changes on PRs. Fixes #2972 Signed-off-by: Shayegan Hooshyari <[email protected]>

jpeach · 2020-10-03T00:20:42Z

@Glyphack Could you add a lint error to this PR to see how the GH action displays it?

Updates: #2976 Signed-off-by: Shayegan Hooshyari <[email protected]>

Glyphack · 2020-10-03T09:44:19Z

@jpeach I added a unused function here is how it looks when action fails:

The implementation of commenting on PRs is not available yet. see: golangci/golangci-lint-action#5

jpeach · 2020-10-04T05:13:24Z

@jpeach I added a unused function here is how it looks when action fails:

The implementation of commenting on PRs is not available yet. see: golangci/golangci-lint-action#5

@stevesloka I think this is a worthwhile improvement, WDYT?

stevesloka

Also will need to remove the steps which run this in the PR GH Action currently: https://github.com/projectcontour/contour/blob/main/.github/workflows/prbuild.yaml#L48-L58

stevesloka · 2020-10-05T19:49:11Z

.github/workflows/golangci-lint.yaml

+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v2
+        with:
+          version: v1.29


Any way we can keep the versions in sync with the Makefile? (https://github.com/projectcontour/contour/blob/main/hack/golangci-lint#L15)

I did not quite understand what you mean. Do you mean is there any way to sync these two versions or syncing it now from v1.29 to v.1.31.0?

I can't think of a great way to have only a single place with the version. We might just have to live with it in multiple places like we do with the Go version :(

There's a version in the hacks/golanglint-ci script as well as now in this GH Action PR. So we need to be diligent in keeping the two in in sync. This is the benefit of just leaving the Makefile doing the work and not having a separate action.

Updates: #2976 Signed-off-by: glyphack <[email protected]>

skriss · 2020-10-19T19:00:50Z

Can someone catch me up on projectcontour/.github#3 and this PR? Reading the backscroll, the idea of the .github repo PR was that it would then be available to all org repos, so in theory we shouldn't need this PR. Is that not the case? (I'm not familiar with how the org-level action sharing works).

Glyphack · 2020-10-19T20:16:24Z

Can someone catch me up on projectcontour/.github#3 and this PR? Reading the backscroll, the idea of the .github repo PR was that it would then be available to all org repos, so in theory we shouldn't need this PR. Is that not the case? (I'm not familiar with how the org-level action sharing works).

Me and @jpeach thought that placing workflows under .github directory will be used as default actions, like CONTRIBUTING.md and CODE_OF_CONDUCT.md. But now I searched for it and found this link simply it means having actions under workflow-templates directory will allow you to copy a workflow between org repositories.
I don't know having it under .github directory have same effect or not. If it's not working(your can check whether it's available in workflow templates or not) I can send another PR and fix it.

jpeach · 2020-10-19T20:37:39Z

Can someone catch me up on projectcontour/.github#3 and this PR? Reading the backscroll, the idea of the .github repo PR was that it would then be available to all org repos, so in theory we shouldn't need this PR. Is that not the case? (I'm not familiar with how the org-level action sharing works).

I don't know having it under .github directory have same effect or not. If it's not working(your can check whether it's available in workflow templates or not) I can send another PR and fix it.

Yeh, I thought that .github worked for workflows, but it turns out that knative has custom automation that copies the workflows around. So that was a bust :(

Glyphack · 2020-10-19T20:44:55Z

@jpeach We can still move it into workflow-templates and It works according to github, right?

jpeach · 2020-10-20T01:47:57Z

@jpeach We can still move it into workflow-templates and It works according to github, right?

IIUC, that copies the workflow to each repo, and doesn't update when the template does. I'd be happy if I was wrong about that though :)

skriss

Thanks for the PR @Glyphack!

I'm personally fine with or without this PR, I don't really have a preference either way for if the linting runs as an action or not. I do find the integrated comments from the codecov action annoying and wish we could toggle them off, but this one probably won't be as spammy.

See one specific comment.

.github/workflows/golangci-lint.yaml

jpeach · 2020-10-20T21:14:41Z

I do find the integrated comments from the codecov action annoying and wish we could toggle them off

I also find the codecov annotations annoying when I'm reviewing. There's a toggle to turn them off in the top right of the source diff view.

skriss · 2020-10-20T21:24:14Z

I do find the integrated comments from the codecov action annoying and wish we could toggle them off

I also find the codecov annotations annoying when I'm reviewing. There's a toggle to turn them off in the top right of the source diff view.

~~Where is this toggle?! I still can't find it.~~ Ah, found it. It's in the per-file options (under the ... at the top-right of each file in the diff).

Updates: #2976 Signed-off-by: glyphack <[email protected]>

skriss

Looks reasonable to me!

youngnick

LGTM

.github/workflows: add golangci lint GH action

d212e84

use https://github.com/marketplace/actions/run-golangci-lint to lint code for new changes on PRs. Fixes #2972 Signed-off-by: Shayegan Hooshyari <[email protected]>

Glyphack added 3 commits October 3, 2020 11:38

set golangci action to only check new issues

6215db6

Updates: #2976 Signed-off-by: Shayegan Hooshyari <[email protected]>

Add a unused function to test golangci-lint

af97c6a

Updates: #2976 Signed-off-by: Shayegan Hooshyari <[email protected]>

Merge branch 'main' into add-lint-gh-action

262334e

jpeach mentioned this pull request Oct 4, 2020

Add golangci-lint action for pull requests. projectcontour/.github#2

Closed

stevesloka suggested changes Oct 5, 2020

View reviewed changes

Glyphack added 4 commits October 6, 2020 21:18

Remove lint step from prbuild action

46abedc

Updates: #2976 Signed-off-by: glyphack <[email protected]>

Bump golangci-lint version to v1.31.0

b281e63

Updates: #2976 Signed-off-by: glyphack <[email protected]>

Revert test golangci-lint

bb3ce4f

Updates: #2976 Signed-off-by: glyphack <[email protected]>

Fix only can set minor version for golangci-lint

5853153

Updates: #2976 Signed-off-by: glyphack <[email protected]>

Glyphack requested a review from stevesloka October 7, 2020 07:04

skriss reviewed Oct 20, 2020

View reviewed changes

.github/workflows/golangci-lint.yaml Outdated Show resolved Hide resolved

Glyphack added 2 commits October 21, 2020 10:25

Move lint action info prbuilb.yml

971b2f4

Updates: #2976 Signed-off-by: glyphack <[email protected]>

Merge branch 'main' into add-lint-gh-action

9e4d3cd

skriss approved these changes Oct 21, 2020

View reviewed changes

youngnick approved these changes Oct 22, 2020

View reviewed changes

skriss merged commit df3f1b1 into projectcontour:main Oct 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add golangci lint GH action #2976

add golangci lint GH action #2976

Glyphack commented Oct 2, 2020

codecov bot commented Oct 2, 2020 •

edited

Loading

jpeach commented Oct 3, 2020

Glyphack commented Oct 3, 2020 •

edited

Loading

jpeach commented Oct 4, 2020

stevesloka left a comment

stevesloka Oct 5, 2020

Glyphack Oct 6, 2020

jpeach Oct 6, 2020

stevesloka Oct 7, 2020

skriss commented Oct 19, 2020

Glyphack commented Oct 19, 2020

jpeach commented Oct 19, 2020

Glyphack commented Oct 19, 2020 •

edited

Loading

jpeach commented Oct 20, 2020

skriss left a comment

jpeach commented Oct 20, 2020

skriss commented Oct 20, 2020 •

edited

Loading

skriss left a comment

youngnick left a comment

add golangci lint GH action #2976

add golangci lint GH action #2976

Conversation

Glyphack commented Oct 2, 2020

codecov bot commented Oct 2, 2020 • edited Loading

Codecov Report

jpeach commented Oct 3, 2020

Glyphack commented Oct 3, 2020 • edited Loading

jpeach commented Oct 4, 2020

stevesloka left a comment

Choose a reason for hiding this comment

stevesloka Oct 5, 2020

Choose a reason for hiding this comment

Glyphack Oct 6, 2020

Choose a reason for hiding this comment

jpeach Oct 6, 2020

Choose a reason for hiding this comment

stevesloka Oct 7, 2020

Choose a reason for hiding this comment

skriss commented Oct 19, 2020

Glyphack commented Oct 19, 2020

jpeach commented Oct 19, 2020

Glyphack commented Oct 19, 2020 • edited Loading

jpeach commented Oct 20, 2020

skriss left a comment

Choose a reason for hiding this comment

jpeach commented Oct 20, 2020

skriss commented Oct 20, 2020 • edited Loading

skriss left a comment

Choose a reason for hiding this comment

youngnick left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 2, 2020 •

edited

Loading

Glyphack commented Oct 3, 2020 •

edited

Loading

Glyphack commented Oct 19, 2020 •

edited

Loading

skriss commented Oct 20, 2020 •

edited

Loading