Skip to content

Conversation

@jas88
Copy link
Owner

@jas88 jas88 commented Nov 8, 2025

Summary

  • Merge CodeQL analysis steps back into the main dotnet.yml workflow
  • Remove separate codeql.yml file that was causing the split
  • Add job-level permissions including contents:write required for release uploads
  • Fix NUGET_KEY reference to NUGET_API_KEY
  • Fix matrix.language reference to hardcoded csharp value

Changes

  • Combined both workflows into single dotnet.yml file (restoring functionality from commit 92a6d03)
  • Added proper permissions: actions: read, contents: write, security-events: write
  • Removed redundant separate codeql.yml workflow
  • Updated secret key name for NuGet publishing

Test plan

  • Verify workflow runs successfully on push
  • Confirm CodeQL analysis executes and uploads SARIF results
  • Test release artifact uploads work with contents:write permission
  • Validate NuGet package publishing on tags

High-level PR Summary

This PR consolidates the CodeQL security analysis workflow into the main dotnet build workflow by removing the separate codeql.yml file and merging its steps into dotnet.yml. It also adds necessary permissions (actions:read, contents:write, security-events:write) at the job level to support release uploads and security scanning, and fixes the NuGet API key secret reference from NUGET_KEY to NUGET_API_KEY. The language reference was also hardcoded from a matrix variable to csharp since only a single language is being analyzed.

⏱️ Estimated Review Time: 5-15 minutes

💡 Review Order Suggestion
Order File Path
1 .github/workflows/codeql.yml
2 .github/workflows/dotnet.yml

Need help? Join our Discord

- Merge CodeQL analysis steps back into dotnet.yml workflow
- Remove separate codeql.yml file
- Add job-level permissions including contents:write for release uploads
- Fix matrix.language reference to hardcoded csharp value
- Restores workflow functionality from commit 92a6d03
Copilot AI review requested due to automatic review settings November 8, 2025 23:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR consolidates the CodeQL security scanning workflow into the main .NET CI/CD pipeline and updates the NuGet API key secret name.

  • Merged the standalone CodeQL workflow into the main dotnet.yml workflow for streamlined CI/CD
  • Updated NuGet deployment to use the correct secret name NUGET_API_KEY instead of NUGET_KEY
  • Added Claude AI assistant local configuration file

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/dotnet.yml Integrated CodeQL scanning steps (initialization, analysis, SARIF filtering/upload) and updated NuGet push secret name
.github/workflows/codeql.yml Removed standalone CodeQL workflow (functionality moved to dotnet.yml)
.claude/settings.local.json Added Claude AI assistant permissions and server configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1 to 18
{
"permissions": {
"allow": [
"Bash(git ls-tree:*)",
"Bash(git cherry-pick:*)",
"Bash(git remote set-url:*)",
"WebFetch(domain:github.com)",
"Bash(/tmp/combined_dotnet.yml)"
],
"deny": [],
"ask": []
},
"enabledMcpjsonServers": [
"github",
"playwright",
"code-index"
]
}
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

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

The .claude/settings.local.json file appears to be a local development configuration file that should likely be excluded from version control. Consider adding .claude/settings.local.json or .claude/*.local.json to .gitignore to prevent local settings from being committed to the repository.

Copilot uses AI. Check for mistakes.
Copy link

@recurseml recurseml bot left a comment

Choose a reason for hiding this comment

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

Review by RecurseML

🔍 Review performed on 41fa58b..674ba68

✨ No bugs found, your code is sparkling clean

✅ Files analyzed, no issues (3)

.claude/settings.local.json
.github/workflows/codeql.yml
.github/workflows/dotnet.yml

jas88 added 2 commits November 8, 2025 17:28
- Add .claude/, *.claude, and .claude-* patterns to .gitignore
- Remove .claude/settings.local.json from git tracking
- Ensures Claude Code configuration files are ignored across all environments
- Remove redundant v3 CodeQL initialization step
- Keep v4 version with proper configuration parameters
- Fixes duplicate Initialize CodeQL steps in workflow
Copilot AI review requested due to automatic review settings November 8, 2025 23:31
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 1 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 19200145989

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 74.35%

Totals Coverage Status
Change from base Build 19198053889: 0.0%
Covered Lines: 994
Relevant Lines: 1325

💛 - Coveralls

@jas88 jas88 merged commit 33118df into main Nov 8, 2025
2 checks passed
@jas88 jas88 deleted the hotfix/github-permissions branch November 8, 2025 23:35
jas88 added a commit that referenced this pull request Nov 9, 2025
Critical fix:
- HicServices#19 ERROR: Implemented IEquatable<DecimalSize> and fixed unsafe equality pattern
  Changed Equals(object) to use pattern matching instead of unsafe cast

Code quality improvements in tests:
- HicServices#22,HicServices#21: Fixed comparison of identical values (use separate references)
- #12,#11: Fixed null argument to Equals (use explicit null cast or Is.Null)
- HicServices#20: Added null-forgiving operator after null assertion
- HicServices#26-HicServices#23: Removed useless object upcasts (declare array as object[] instead)
- #10,HicServices#18,HicServices#17,HicServices#16: Wrapped IDisposable usage in using statements for guaranteed cleanup

All changes in test files only. Production code quality improved via IEquatable.
Build: 0 warnings, 0 errors. Tests: 377/377 passing.
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.

3 participants