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

Fixing print of lint errors in golangci-lint action #246

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

patrickhuie19
Copy link
Contributor

@patrickhuie19 patrickhuie19 commented Nov 16, 2023

Previously the lint artifact was in xml, so the step to find errors omits the file name when logging to the action. This means that devs still have to pull the artifact down locally to fix lint errors, which I think is exactly what the step was trying to prevent.

Example:
https://github.com/smartcontractkit/chainlink-relay/actions/runs/6896336069/job/18762232176?pr=248

I saw two paths forward:

  • Change the output encoding to json, which is easier to parse than xml
  • Write a more complex parser for the xml to include the file name with the error

I chose the first, with the caveat that golangci-lint allows multiple output encodings/files, which is necessary as sonar only allows for golangci-lint artifacts with the checkstyle format: https://docs.sonarsource.com/sonarqube/latest/analyzing-source-code/importing-external-issues/external-analyzer-reports/#list-of-properties

The main thing we have to make sure of here is that no integration with sonar is broken.
Verified with https://github.com/smartcontractkit/chainlink-relay/pull/246/checks?check_run_id=18763287499 on commit 3328779 of this PR, which has parity with the same lint errors on #248

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants