Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce golang ci lint action #723

Closed
wants to merge 4 commits into from

Conversation

tungbq
Copy link

@tungbq tungbq commented Jun 24, 2023

Resolves: #718

Background (mentioned in the issue #718 ):

Golang lint is a fast Go linters runner. It runs linters in parallel, uses caching, supports yaml config. It’s used to make sure code adheres to common Go practices. It supports enable many different types of linters.

Changes:

  • Add golangci-lint-action to the CI workflows as it's the official GitHub action for golangci-lint from its authors. The action runs golangci-lint and reports issues from linters.
  • Name the new workflow as golangci-lint in .github/workflows/go-lint.yml
  • Current behaviour:
    • Run golangci-lint on every PR
    • Report only new issue generated from the PR
    • Beside of PR events, there has an option in the trigger event to allow us manually run the workflow if needed (workflow_dispatch:)

Testing:

@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2023

Codecov Report

Merging #723 (79beaf2) into main (0f3c4ae) will not change coverage.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##             main     #723   +/-   ##
=======================================
  Coverage   46.59%   46.59%           
=======================================
  Files          40       40           
  Lines        2232     2232           
=======================================
  Hits         1040     1040           
  Misses       1105     1105           
  Partials       87       87           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR decreases the code coverage for 17.04%. Could you take a look?

uses: golangci/golangci-lint-action@v3
with:
version: latest
only-new-issues: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is the first time we introduce the CI lint, we should sweep the code for any existing issues and resolve them. Therefore, we need to set only-new-issues to false, which is the default value.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated. Using the default value to catch all the existing issues

- uses: actions/setup-go@v4
with:
go-version: '1.20'
cache: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use a fixed version of golang, which is recommended, we can resuse the cache for perf and cost.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion. I updated in the latest commit

@JeyJeyGao
Copy link
Contributor

This issue that caused by Zot V2.0.0-rc.5 update will be resolved in #727

This PR decreases the code coverage for 17.04%. Could you take a look?

@tungbq tungbq force-pushed the add-golang-ci-lint branch from d831cb4 to 79beaf2 Compare June 28, 2023 14:56
@tungbq
Copy link
Author

tungbq commented Jun 28, 2023

This issue that caused by Zot V2.0.0-rc.5 update will be resolved in #727

This PR decreases the code coverage for 17.04%. Could you take a look?

Thanks for pointing this @JeyJeyGao

@tungbq tungbq requested a review from shizhMSFT June 28, 2023 15:00
Copy link

This PR is stale because it has been opened for 45 days with no activity. Remove stale label or comment. Otherwise, it will be closed in 30 days.

@github-actions github-actions bot added the Stale label Mar 22, 2024
Copy link

PR closed due to no activity in the past 30 days.

@github-actions github-actions bot closed this Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants