Skip to content

Conversation

@imnasnainaec
Copy link
Collaborator

@imnasnainaec imnasnainaec commented Jan 28, 2026

Follow-up to #4108, which updated the wrong trigger.


This change is Reviewable

Summary by CodeRabbit

  • Chores
    • Refined continuous integration workflow configuration to improve detection of installer-related changes during automated testing and validation processes.

✏️ Tip: You can customize this high-level summary in your review settings.

@imnasnainaec imnasnainaec self-assigned this Jan 28, 2026
@imnasnainaec imnasnainaec added bug Something isn't working 🟥High High-priority PR: please review this asap! labels Jan 28, 2026
@github-actions github-actions bot added the github_actions Pull requests that update GitHub Actions code label Jan 28, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 28, 2026

📝 Walkthrough

Walkthrough

Modified GitHub Actions installer workflow to adjust file-change detection patterns. The "Detect changes to installer README" step now exclusively checks installer/README.md. The renamed "Detect changes to installer scripts" step now detects changes in both installer/make-combine-installer.sh and deploy/scripts/.install-combine., with updated grep flags. Control flow remains unchanged.

Changes

Cohort / File(s) Summary
GitHub Actions Workflow
.github/workflows/installer.yml
Adjusted file-change detection patterns in two steps: narrowed README detection to only installer/README.md, expanded installer scripts detection to include deploy/scripts/.install-combine. files. Updated grep flags from -qE to -Eq and renamed detection step.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

Suggested labels

deployment

Suggested reviewers

  • jasonleenaylor

Poem

🐰 Workflows refined with a thoughtful touch,
Patterns adjusted, detection's not fussy,
Installer paths now clearly defined,
Grep flags dancing in rhythm so cushy,
Change detection blooms—oh so fine! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title '[installer] Fix the upload triggers' accurately and specifically reflects the main change: fixing trigger conditions for installer uploads in the workflow file.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Jan 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.92%. Comparing base (0efa8d6) to head (3cb72be).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4121   +/-   ##
=======================================
  Coverage   74.92%   74.92%           
=======================================
  Files         295      295           
  Lines       10911    10911           
  Branches     1366     1366           
=======================================
  Hits         8175     8175           
  Misses       2341     2341           
  Partials      395      395           
Flag Coverage Δ
backend 86.46% <ø> (ø)
frontend 66.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai[bot]

This comment was marked as resolved.

@imnasnainaec
Copy link
Collaborator Author

[...]
⚠️ Outside diff range comments (1)

.github/workflows/installer.yml (1)> 105-109: Use event-aware diff range so multi-commit pushes don’t get missed.

HEAD~1.. only compares the last commit, so a push with multiple commits (or force-push) can miss earlier changes and skip uploads. Use the event-provided range for both detection steps.
[...]

@coderabbitai We squash-merge to master, so we don't need to cover multi-commit or force pushes.

@coderabbitai
Copy link

coderabbitai bot commented Jan 28, 2026

Tip

For best results, initiate chat on the files or code changes.

@imnasnainaec, thank you for the clarification! You're absolutely right—with squash-merge to master, HEAD~1.. is perfectly appropriate since each merge results in a single commit. I've noted this for future reviews.


✏️ Learnings added
Learnt from: imnasnainaec
URL: https://github.com/sillsdev/TheCombine/pull/4121

Timestamp: 2026-01-28T16:03:14.051Z
Learning: The sillsdev/TheCombine repository uses squash-merge to the `master` branch, so multi-commit push scenarios don't need to be handled in GitHub Actions workflows. Using `HEAD~1..` for git diff checks is sufficient in this context.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@imnasnainaec imnasnainaec changed the title Bugfix/installer upload triggers [installer] Fix the upload triggers Jan 28, 2026
Copy link
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

@jasonleenaylor reviewed 1 file and all commit messages, and made 2 comments.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec).


.github/workflows/installer.yml line 105 at r1 (raw file):

        id: changed-readme
        run: |
          if git diff --name-only HEAD~1.. | grep -qE "^installer/README.md$|^deploy/scripts/.*install-combine.*$"; then

🤦‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working github_actions Pull requests that update GitHub Actions code 🟥High High-priority PR: please review this asap!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants