guibranco / BancosBrasileiros-MergeTool

๐Ÿ‡ง๐Ÿ‡ท ๐Ÿฆ ๐Ÿ“‹ Brazilian banks: MergeTool - The C# .NET tool used to merge and keep data from the Bancos Brasileiros repository updated
https://guibranco.github.io/BancosBrasileiros-MergeTool/
MIT License
7 stars 0 forks source link

Add Deep Source Workflow for Enhanced Code Quality Analysis #146

Closed guibranco closed 3 months ago

guibranco commented 3 months ago

Description


Changes walkthrough ๐Ÿ“

Relevant files
Enhancement
deep-source.yml
Add Deep Source GitHub Actions Workflow for Code Analysis

.github/workflows/deep-source.yml
  • Added a new GitHub Actions workflow for Deep Source.
  • Configured the workflow to run on push and pull request events.
  • Included steps for installing the DeepSource scanner and setting up
    .NET.
  • Added steps for building and analyzing code coverage.
  • +45/-0   
    penify-dev[bot] commented 3 months ago

    PR Review ๐Ÿ”

    โฑ๏ธ Estimated effort to review [1-5] 2, because the new workflow is straightforward and primarily involves configuration without complex logic.
    ๐Ÿงช Relevant tests No
    โšก Possible issues No
    ๐Ÿ”’ Security concerns Sensitive information exposure: The workflow uses a secret (DEEPSOURCE_DSN) which should be managed carefully to avoid accidental exposure in logs or outputs.
    penify-dev[bot] commented 3 months ago

    PR Code Suggestions โœจ

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Specify a version for the DeepSource CLI installation to ensure stability ___ **Consider using a specific version for the DeepSource CLI installation to ensure
    consistency and avoid potential breaking changes in future releases.** [.github/workflows/deep-source.yml [22]](https://github.com/guibranco/BancosBrasileiros-MergeTool/pull/146/files#diff-dd5be344ce9fee02de9a8b1b1f9308714143f4fb6b708c36f1da39a7f57b149fR22-R22) ```diff -curl https://deepsource.io/cli | sh +curl -sSL https://deepsource.io/cli/v1.0.0/install.sh | sh ```
    Suggestion importance[1-10]: 8 Why: Specifying a version for the DeepSource CLI installation enhances stability and prevents issues from future updates, making it a valuable suggestion.
    8
    Pin the .NET version to a specific release to prevent unexpected changes ___ **The dotnet-version should be pinned to a specific version instead of using a wildcard to
    avoid unexpected behavior due to future updates.** [.github/workflows/deep-source.yml [27]](https://github.com/guibranco/BancosBrasileiros-MergeTool/pull/146/files#diff-dd5be344ce9fee02de9a8b1b1f9308714143f4fb6b708c36f1da39a7f57b149fR27-R27) ```diff -dotnet-version: '8.0.x' +dotnet-version: '8.0.100' ```
    Suggestion importance[1-10]: 8 Why: Pinning the .NET version to a specific release helps ensure that the build environment remains consistent, which is crucial for avoiding unexpected behavior.
    8
    Add a cleanup step to remove generated files after the analysis ___ **Consider adding a step to clean up any generated files after the analysis to keep the
    environment tidy.** [.github/workflows/deep-source.yml [45]](https://github.com/guibranco/BancosBrasileiros-MergeTool/pull/146/files#diff-dd5be344ce9fee02de9a8b1b1f9308714143f4fb6b708c36f1da39a7f57b149fR45-R45) ```diff -./bin/deepsource report --analyzer test-coverage --key csharp --value-file Tests/Results/coverage.cobertura.xml +./bin/deepsource report --analyzer test-coverage --key csharp --value-file Tests/Results/coverage.cobertura.xml && rm -rf Tests/Results/* ```
    Suggestion importance[1-10]: 6 Why: Adding a cleanup step is a good practice for maintaining a tidy environment, but it is not critical to the functionality of the workflow, hence the moderate score.
    6
    Possible issue
    Add a check to ensure the DEEPSOURCE_DSN secret is set before execution ___ **Ensure that the DEEPSOURCE_DSN secret is set before running the DeepSource report to avoid
    runtime errors.** [.github/workflows/deep-source.yml [41]](https://github.com/guibranco/BancosBrasileiros-MergeTool/pull/146/files#diff-dd5be344ce9fee02de9a8b1b1f9308714143f4fb6b708c36f1da39a7f57b149fR41-R41) ```diff +if: ${{ secrets.DEEPSOURCE_DSN }} # Ensure the secret is set DEEPSOURCE_DSN: ${{ secrets.DEEPSOURCE_DSN }} ```
    Suggestion importance[1-10]: 5 Why: While checking for the existence of the `DEEPSOURCE_DSN` secret is important, the suggested implementation does not correctly integrate the check into the workflow syntax, making it less effective.
    5
    sonarcloud[bot] commented 3 months 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