Update GitHub Actions workflow for build and analysis - #19
Merged
Conversation
Signed-off-by: Adonis Deliannis <blizzardo1@blizzeta.net>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the Windows build-and-analyze GitHub Actions workflow by pinning action versions to specific SHAs, adjusting cache locations to use the runner’s temp directory, updating SonarQube project key and build command, and simplifying branch triggers. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The SonarQube cache path was changed from
~\.sonar\cacheto${{ runner.temp }}\cache, but the scanner itself will still default to using the.sonardirectory under the user home; consider aligning the cache path with the actual directory the scanner uses so the cache is effective. - The SonarQube project key is now hard-coded to
Blizzardo1_SharpSDL3_2f21dd8f-7c45-448a-88e7-addecd7a4565; consider moving this to a reusable variable or secret (e.g., workflow env or repository secret) to make future changes safer and reduce leakage of internal identifiers. - The build step was changed from
dotnet build SDL3\SharpSDL3.csprojtodotnet build, which may cause additional projects or configurations to be built; if that’s not intended, consider explicitly targeting the desired solution or project file.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The SonarQube cache path was changed from `~\.sonar\cache` to `${{ runner.temp }}\cache`, but the scanner itself will still default to using the `.sonar` directory under the user home; consider aligning the cache path with the actual directory the scanner uses so the cache is effective.
- The SonarQube project key is now hard-coded to `Blizzardo1_SharpSDL3_2f21dd8f-7c45-448a-88e7-addecd7a4565`; consider moving this to a reusable variable or secret (e.g., workflow env or repository secret) to make future changes safer and reduce leakage of internal identifiers.
- The build step was changed from `dotnet build SDL3\SharpSDL3.csproj` to `dotnet build`, which may cause additional projects or configurations to be built; if that’s not intended, consider explicitly targeting the desired solution or project file.
## Individual Comments
### Comment 1
<location path=".github/workflows/build.yml" line_range="46" />
<code_context>
- dotnet build SDL3\SharpSDL3.csproj
- .\.sonar\scanner\dotnet-sonarscanner end /d:sonar.token="${{ secrets.SONAR_TOKEN }}"
+ ${{ runner.temp }}\scanner\dotnet-sonarscanner begin /k:"Blizzardo1_SharpSDL3_2f21dd8f-7c45-448a-88e7-addecd7a4565" /d:sonar.token="${{ secrets.SONAR_TOKEN }}" /d:sonar.host.url="${{ secrets.SONAR_HOST_URL }}"
+ dotnet build
+ ${{ runner.temp }}\scanner\dotnet-sonarscanner end /d:sonar.token="${{ secrets.SONAR_TOKEN }}"
</code_context>
<issue_to_address>
**suggestion:** Using a bare `dotnet build` can be brittle if the repo layout changes.
The previous workflow built `SDL3\SharpSDL3.csproj` explicitly; now it relies on whatever the working directory treats as the default project/solution. If more projects/solutions are added at the repo root or that default changes, this step could build the wrong thing or fail. Please specify the intended project/solution in the `dotnet build` command to keep the workflow resilient to repo layout changes.
```suggestion
dotnet build SDL3\SharpSDL3.csproj
```
</issue_to_address>
### Comment 2
<location path=".github/workflows/build.yml" line_range="40" />
<code_context>
run: |
- New-Item -Path .\.sonar\scanner -ItemType Directory
- dotnet tool update dotnet-sonarscanner --tool-path .\.sonar\scanner
+ New-Item -Path ${{ runner.temp }}\scanner -ItemType Directory
+ dotnet tool update dotnet-sonarscanner --tool-path ${{ runner.temp }}\scanner
- name: Build and analyze
</code_context>
<issue_to_address>
**issue:** Creating the scanner directory unconditionally may fail if the cache already restored it.
If `actions/cache` restores `${{ runner.temp }}\scanner`, the directory will already exist and `New-Item` (without `-Force`) will throw, breaking the job on cache hits. Please either check for existence before creating it or add `-Force` so this step is safe for both cached and non-cached runs.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Update the GitHub Actions build workflow to use pinned action versions, adjust SonarQube caching to use the runner temp directory, and align the scanner configuration and build command with the current Sonar project.
Build: