Skip to content

Conversation

@graycreate
Copy link
Member

Summary

  • Merge code-quality.yml, ios-build-test.yml, and pr-validation.yml into a unified ci.yml
  • All 6 jobs run in parallel with unified trigger conditions
  • Reduces workflow file count from 3 to 1 for easier maintenance

Jobs

Job Description Runner
lint SwiftLint checks ubuntu-latest
swiftformat SwiftFormat code style checks macos-26
pr-size PR size labeling ubuntu-latest
check-commits Conventional commit validation ubuntu-latest
build-and-test Build and run tests macos-26
code-coverage Test coverage reporting macos-26

Changes

  • New: .github/workflows/ci.yml - Consolidated workflow
  • Deleted: .github/workflows/code-quality.yml
  • Deleted: .github/workflows/ios-build-test.yml
  • Deleted: .github/workflows/pr-validation.yml
  • Kept: .github/workflows/release.yml (different trigger: version changes)
  • Kept: .github/workflows/dependency-update.yml (scheduled workflow)

Reference

Based on grayverse/ULPB-iOS#106

Test plan

  • Verify all jobs trigger on PR
  • Confirm jobs run in parallel
  • Check concurrency control works (cancel old runs)

🤖 Generated with Claude Code

Merge `code-quality.yml`, `ios-build-test.yml`, and `pr-validation.yml`
into a unified `ci.yml` workflow. All 6 jobs run in parallel with
unified trigger conditions and concurrency control.

Jobs:
- lint: SwiftLint checks
- swiftformat: SwiftFormat code style checks
- pr-size: PR size labeling
- check-commits: Conventional commit validation
- build-and-test: Build and run tests
- code-coverage: Test coverage reporting

This reduces workflow file count from 3 to 1 for easier maintenance.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings November 30, 2025 00:43
Copilot finished reviewing on behalf of graycreate November 30, 2025 00:45
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 three separate GitHub Actions workflow files (code-quality.yml, ios-build-test.yml, and pr-validation.yml) into a single unified ci.yml workflow for improved maintainability. The consolidation preserves all 6 original jobs (lint, swiftformat, pr-size, check-commits, build-and-test, code-coverage) while unifying trigger conditions and adding concurrency control.

  • Merges 3 workflow files into 1 unified CI workflow
  • Enables all jobs to run in parallel with appropriate conditions
  • Adds concurrency control to cancel outdated workflow runs

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/ci.yml New consolidated workflow combining all CI jobs with unified triggers and concurrency control
.github/workflows/pr-validation.yml Removed - functionality moved to ci.yml (lint, pr-size, check-commits jobs)
.github/workflows/ios-build-test.yml Removed - functionality moved to ci.yml (build-and-test job)
.github/workflows/code-quality.yml Removed - functionality moved to ci.yml (swiftformat, code-coverage jobs)

Comment on lines +125 to +131
for (const label of labels.data) {
if (label.name.startsWith('size/')) {
await github.rest.issues.removeLabel({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pr.number,
name: label.name
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

Variable name collision: the loop variable label on line 125 shadows the label variable defined on line 110. This will cause the size label calculation to be overwritten during the loop. Consider renaming the loop variable to something like existingLabel to avoid this conflict.

for (const existingLabel of labels.data) {
  if (existingLabel.name.startsWith('size/')) {
    await github.rest.issues.removeLabel({
      owner: context.repo.owner,
      repo: context.repo.repo,
      issue_number: pr.number,
      name: existingLabel.name
    });
  }
}
Suggested change
for (const label of labels.data) {
if (label.name.startsWith('size/')) {
await github.rest.issues.removeLabel({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pr.number,
name: label.name
for (const existingLabel of labels.data) {
if (existingLabel.name.startsWith('size/')) {
await github.rest.issues.removeLabel({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pr.number,
name: existingLabel.name

Copilot uses AI. Check for mistakes.
xcrun xccov view --report --json "$RESULT_BUNDLE" > coverage.json || echo '{}' > coverage.json

# Extract coverage percentage with fallback
COVERAGE=$(cat coverage.json | jq -r '.lineCoverage // 0' | awk '{printf "%.2f", $1 * 100}')
Copy link

Copilot AI Nov 30, 2025

Choose a reason for hiding this comment

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

Useless use of cat: The command cat coverage.json | jq can be simplified to jq '...' coverage.json which is more efficient and follows best practices by avoiding an unnecessary pipe.

COVERAGE=$(jq -r '.lineCoverage // 0' coverage.json | awk '{printf "%.2f", $1 * 100}')
Suggested change
COVERAGE=$(cat coverage.json | jq -r '.lineCoverage // 0' | awk '{printf "%.2f", $1 * 100}')
COVERAGE=$(jq -r '.lineCoverage // 0' coverage.json | awk '{printf "%.2f", $1 * 100}')

Copilot uses AI. Check for mistakes.
@graycreate graycreate merged commit 69f9222 into main Nov 30, 2025
10 of 12 checks passed
@graycreate graycreate deleted the ci/consolidate-workflows branch November 30, 2025 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants