Skip to content

Introduce a new CI job for building the phrase db - #737

Merged
lukhnos merged 2 commits into
masterfrom
dev/build-data-ci-job
Nov 13, 2025
Merged

Introduce a new CI job for building the phrase db#737
lukhnos merged 2 commits into
masterfrom
dev/build-data-ci-job

Conversation

@lukhnos

@lukhnos lukhnos commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

User description

This makes sure that we build the entire phrase db upon any changes in Source/Data/**.


PR Type

Enhancement, Other


Description

  • Add CI workflow to build phrase DB

  • Trigger on Source/Data path changes

  • Set up Python 3.12 on macOS-15

  • Run make in Source/Data directory


Diagram Walkthrough

flowchart LR
  trigger["Push/PR to 'Source/Data/**'"] 
  setup["Checkout + Setup Python 3.12 (macOS-15)"]
  build["Run 'make' in 'Source/Data' to build phrase DB"]
  trigger -- "starts job" --> setup
  setup -- "executes" --> build
Loading

File Walkthrough

Relevant files
Configuration changes
continuous-build-data.yml
New GitHub Actions workflow for phrase DB build                   

.github/workflows/continuous-build-data.yml

  • Add GitHub Actions workflow named Build
  • Trigger on push/PR affecting Source/Data/**
  • Setup Python 3.12 on macOS-15 runner
  • Run make in Source/Data to build phrase DB
+21/-0   

@lukhnos
lukhnos requested review from Copilot and zonble November 13, 2025 03:35
@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported.

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Runner Choice

Using macOS-15 may be slower and more limited in concurrency/availability; consider ubuntu-latest unless macOS is required for the build.

runs-on: macOS-15
Caching

No dependency caching is configured; adding actions/cache for pip/build artifacts could significantly speed up the job.

- uses: actions/setup-python@v6
  with:
    python-version: '3.12'
- name: Build phrase database
Trigger Scope

The workflow only triggers on changes under Source/Data; updates to build tooling (Makefile, workflow, or shared scripts) won't run this job—consider broadening paths or adding workflow_dispatch.

  paths:
    - 'Source/Data/**'
pull_request:
  paths:
    - 'Source/Data/**'

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Use stable runner image

Pin the runner to a stable GA release to avoid unexpected breakages from preview
images. Use a specific, supported macOS runner label or switch to a Linux runner if
macOS is not required.

.github/workflows/continuous-build-data.yml [13]

-runs-on: macOS-15
+runs-on: macos-14
Suggestion importance[1-10]: 7

__

Why: Pinning runs-on from macOS-15 to a stable GA like macos-14 reduces breakage risk; this is accurate and relevant but not critical to functionality.

Medium
General
Fail fast with strict shell

Explicitly set the shell to bash and enable -euo pipefail to fail fast on errors and
unset variables. This prevents silent failures during the build.

.github/workflows/continuous-build-data.yml [19-21]

 - name: Build phrase database
-  run: make
+  shell: bash
+  run: |
+    set -euo pipefail
+    make
   working-directory: Source/Data
Suggestion importance[1-10]: 7

__

Why: Enabling bash with set -euo pipefail improves reliability by catching errors early; it’s accurate and moderately impactful without altering intended behavior.

Medium
Cache Python dependencies

Add actions/cache for pip to prevent cache misses and speed up builds. This also
reduces flakiness due to rate limits and network issues when installing
dependencies.

.github/workflows/continuous-build-data.yml [15-18]

 - uses: actions/checkout@v5
 - uses: actions/setup-python@v6
   with:
     python-version: '3.12'
+- name: Cache pip
+  uses: actions/cache@v4
+  with:
+    path: ~/.cache/pip
+    key: ${{ runner.os }}-pip-${{ hashFiles('Source/Data/requirements*.txt') }}
+    restore-keys: |
+      ${{ runner.os }}-pip-
Suggestion importance[1-10]: 6

__

Why: Adding a pip cache after setup-python is a valid optimization that can speed builds; it’s beneficial but not essential and depends on having requirements files.

Low

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical Issues

Error handling: The workflow doesn't verify the make command succeeds. If the phrase database build fails, the CI job will still pass, making the check ineffective. Consider adding validation that expected output files exist after the build completes.

Workflow naming: The generic name "Build" may conflict with existing workflows and cause confusion in the GitHub UI.

Comment thread .github/workflows/continuous-build-data.yml
Comment thread .github/workflows/continuous-build-data.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 introduces a new GitHub Actions workflow to automatically build the phrase database whenever changes are made to files in Source/Data/**, ensuring data integrity through continuous integration.

  • Adds workflow triggered on push/PR affecting Source/Data/** files
  • Configures macOS-15 runner with Python 3.12 setup
  • Executes the Makefile in Source/Data to build phrase database artifacts

Comment thread .github/workflows/continuous-build-data.yml
@lukhnos
lukhnos merged commit a695d10 into master Nov 13, 2025
6 checks passed
@lukhnos
lukhnos deleted the dev/build-data-ci-job branch November 14, 2025 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants