tektoncd / results

Long term storage of execution results.
Apache License 2.0
77 stars 73 forks source link

Add golang ci linter #465

Closed AndrienkoAleksandr closed 1 year ago

AndrienkoAleksandr commented 1 year ago

Changes

Out of the scope this pr.

Fixing all lint issues.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you review them:

Release Notes

tekton-robot commented 1 year ago

Hi @AndrienkoAleksandr. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.
AndrienkoAleksandr commented 1 year ago

Looks like github actions are disabled in the github project configuration.

AndrienkoAleksandr commented 1 year ago

@dibyom do I have chance to merge this pull request ?

tekton-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom

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: - ~~[OWNERS](https://github.com/tektoncd/results/blob/main/OWNERS)~~ [dibyom] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
khrm commented 1 year ago

/hold

Let's wait for response before merging.

AndrienkoAleksandr commented 1 year ago

@dibyom We aren't using workflows/GH actions for linting in the other Tekton projects. Shouldn't we take a consistent approach and implement linting in the same way?

@khrm, Strange I see the same github action for tekton-chains: https://github.com/tektoncd/chains/blob/main/.github/workflows/lint.yaml and https://github.com/tektoncd/chains/actions/runs/4952394643/jobs/8858626717?pr=801

For tekton-pipeline I see lint config https://github.com/tektoncd/pipeline/blob/main/.golangci.yml, but I don't see where is ci configs.

AndrienkoAleksandr commented 1 year ago

Looks like ci dogfooding configuration stored here https://github.com/tektoncd/plumbing ...

AndrienkoAleksandr commented 1 year ago

I see there is pipelinerun https://github.com/tektoncd/plumbing/blob/main/tekton/ci/jobs/tekton-golang-lint.yaml with lint

AndrienkoAleksandr commented 1 year ago

Also I see usage of this lint check for plumbing project https://github.com/tektoncd/plumbing/blob/main/tekton/ci/repos/plumbing/template.yaml#LL102C9-L102C9 . But when I open any pull request from this project, I see this lint ci check is absent. So I'm confused how it works...

khrm commented 1 year ago

@AndrienkoAleksandr Please check this pr: https://github.com/tektoncd/results/pull/493

We generally use Makefile for Triggers, Pipelines, etc.

khrm commented 1 year ago

Workflows are disable because of some issue.

AndrienkoAleksandr commented 1 year ago

Close on favor https://github.com/tektoncd/results/pull/493