TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols
Other
9.51k stars 1.07k forks source link

[TT-10532] Fix golangci-lint step to exit 0 #6369

Closed titpetric closed 1 month ago

titpetric commented 1 month ago

User description

The change introduced in #6350 changed the exit-code incorrectly. Added comment.


PR Type

bug fix, enhancement


Description


Changes walkthrough ๐Ÿ“

Relevant files
Bug fix
ci-tests.yml
Fix golangci-lint exit code and add explanatory comments 

.github/workflows/ci-tests.yml
  • Added comments to explain the need for issues-exit-code=0 in
    golangci-lint step.
  • Changed issues-exit-code from 1 to 0 in golangci-lint step.
  • +3/-1     

    ๐Ÿ’ก PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    github-actions[bot] commented 1 month ago

    PR Reviewer Guide ๐Ÿ”

    โฑ๏ธ Estimated effort to review [1-5] 2
    ๐Ÿงช Relevant tests No
    ๐Ÿ”’ Security concerns No
    โšก Key issues to review None
    github-actions[bot] commented 1 month ago

    API Changes

    no api changes detected
    github-actions[bot] commented 1 month ago

    PR Code Suggestions โœจ

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add error handling for the output redirection of golangci-lint ___ **Ensure that the redirection of golangci-lint output to golanglint.xml is handled correctly
    in case of any errors during the linting process. It's good practice to check if the file
    was successfully written to avoid silent failures in the CI pipeline.** [.github/workflows/ci-tests.yml [110]](https://github.com/TykTechnologies/tyk/pull/6369/files#diff-03609cb60b0c6e92fb771eb8787d6722b8c31ca4c03eabc788e147acd8c6fb43R110-R110) ```diff -$(go env GOPATH)/bin/golangci-lint run --out-format checkstyle --issues-exit-code=0 --new-from-rev=origin/${{ github.base_ref }} ./... > golanglint.xml +$(go env GOPATH)/bin/golangci-lint run --out-format checkstyle --issues-exit-code=0 --new-from-rev=origin/${{ github.base_ref }} ./... > golanglint.xml || echo "Error: golangci-lint failed to write to golanglint.xml" ```
    Suggestion importance[1-10]: 9 Why: The suggestion adds error handling to ensure that any issues with writing the output file are caught, which is crucial for detecting silent failures in the CI pipeline. This enhances the robustness of the CI process.
    9
    Maintainability
    Improve the comment for clarity and understanding of the issues-exit-code=0 setting ___ **Consider using a more descriptive comment to explain why issues-exit-code=0 is necessary.
    This will help future maintainers understand the reasoning behind this configuration
    choice, especially in complex workflows where non-zero exit codes are typically used to
    indicate failure.** [.github/workflows/ci-tests.yml [105-106]](https://github.com/TykTechnologies/tyk/pull/6369/files#diff-03609cb60b0c6e92fb771eb8787d6722b8c31ca4c03eabc788e147acd8c6fb43R105-R106) ```diff -# golangci-lint actions *require* issues-exit-code=0 to pass data along to sonarcloud -# rather than erroring out on github issues directly with out-format github. +# Set `issues-exit-code=0` to ensure golangci-lint does not fail the CI step even if issues are found. +# This allows the results to be passed to sonarcloud for further analysis without breaking the CI pipeline. ```
    Suggestion importance[1-10]: 8 Why: The suggestion improves the clarity of the comment, making it easier for future maintainers to understand the reasoning behind the configuration. This is important for maintainability and readability.
    8
    sonarcloud[bot] commented 1 month ago

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    0.0% Coverage on New Code
    0.0% Duplication on New Code

    See analysis details on SonarCloud