Skip to content

Conversation

@jongalloway
Copy link
Collaborator

This pull request introduces an automated code formatting job to the GitHub Actions workflow and updates dependencies for several jobs to ensure they only run after successful or skipped formatting. The changes aim to improve code quality and streamline the CI/CD pipeline by integrating dotnet format.

New auto-formatting job:

  • Addition of auto-format job: A new job was added to format code using dotnet format for both the main solution (NLWebNet.sln) and the AspireDemo solution (NLWebNet.AspireDemo.sln). It includes steps for checking out the code, setting up .NET, restoring dependencies, formatting the code, checking for formatting changes, and committing them if necessary. This job runs only on push events and excludes the main branch. ([.github/workflows/build.ymlL124-R178](https://github.com/nlweb-ai/nlweb-net/pull/45/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721L124-R178))

Updates to existing jobs:

  • Dependency on auto-format job: Jobs such as code-quality, security-scan, package-validation, docker-build, and dotnet-container-build now depend on the auto-format job. They are configured to run only if the auto-format job succeeds or is skipped. ([[1]](https://github.com/nlweb-ai/nlweb-net/pull/45/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721L148-R207), [[2]](https://github.com/nlweb-ai/nlweb-net/pull/45/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721L179-R234), [[3]](https://github.com/nlweb-ai/nlweb-net/pull/45/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721L272-R327), [[4]](https://github.com/nlweb-ai/nlweb-net/pull/45/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721L316-R371))
  • Enhanced formatting verification: The code-quality job now explicitly verifies formatting for both solutions (NLWebNet.sln and NLWebNet.AspireDemo.sln) using dotnet format --verify-no-changes. This ensures consistent code style across the repository. ([.github/workflows/build.ymlL148-R207](https://github.com/nlweb-ai/nlweb-net/pull/45/files#diff-5c3fa597431eda03ac3339ae6bf7f05e1a50d6fc7333679ec38e21b337cb6721L148-R207))

@jongalloway jongalloway requested a review from Copilot July 2, 2025 04:43
Copy link
Contributor

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 adds an automated formatting job and makes existing CI jobs depend on its outcome to ensure consistent code style.

  • Introduces an auto-format GitHub Actions job that runs dotnet format on both solutions and commits any changes.
  • Updates code-quality, security-scan, package-validation, docker-build, and dotnet-container-build jobs to wait for the auto-format job to succeed or be skipped.
  • Enhances formatting verification in the code-quality job to cover the AspireDemo solution.
Comments suppressed due to low confidence (3)

.github/workflows/build.yml:127

  • The auto-format job pushes formatting commits on every push, which may retrigger the workflow in a loop. Consider adding a guard (e.g., checking github.actor != 'github-actions[bot]' or inspecting the commit message) to skip runs triggered by the formatting action.
    if: needs.check-changes.outputs.should-skip != 'true' && github.event_name == 'push' && github.ref != 'refs/heads/main'

.github/workflows/build.yml:178

  • [nitpick] This complex if expression is duplicated across multiple jobs. Extracting it into a reusable condition (for example via a workflow-level environment variable or a shared composite action) would reduce duplication and simplify future updates.
    if: needs.check-changes.outputs.should-skip != 'true' && always() && (needs.auto-format.result == 'success' || needs.auto-format.result == 'skipped')

.github/workflows/build.yml:171

  • [nitpick] Using git add . stages all working directory changes, which could include unrelated files. Consider adding only the files modified by dotnet format (e.g., using git diff --name-only --diff-filter=ACM | xargs git add) to avoid accidental commits.
        git add .

@jongalloway
Copy link
Collaborator Author

@copilot Please review these comments and fix if necessary:

.github/workflows/build.yml:127

  • The auto-format job pushes formatting commits on every push, which may retrigger the workflow in a loop. Consider adding a guard (e.g., checking github.actor != 'github-actions[bot]' or inspecting the commit message) to skip runs triggered by the formatting action.
    if: needs.check-changes.outputs.should-skip != 'true' && github.event_name == 'push' && github.ref != 'refs/heads/main'

.github/workflows/build.yml:178

  • [nitpick] This complex if expression is duplicated across multiple jobs. Extracting it into a reusable condition (for example via a workflow-level environment variable or a shared composite action) would reduce duplication and simplify future updates.
    if: needs.check-changes.outputs.should-skip != 'true' && always() && (needs.auto-format.result == 'success' || needs.auto-format.result == 'skipped')

.github/workflows/build.yml:171

  • [nitpick] Using git add . stages all working directory changes, which could include unrelated files. Consider adding only the files modified by dotnet format (e.g., using git diff --name-only --diff-filter=ACM | xargs git add) to avoid accidental commits.
        git add .

@jongalloway jongalloway merged commit 7f04a03 into main Jul 2, 2025
9 checks passed
@jongalloway jongalloway deleted the format-on-build branch July 4, 2025 22:00
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.

2 participants