Document CI/CD pipeline#171
Conversation
There was a problem hiding this comment.
Mention job sharding and the status check job afterwards for UI app tests.
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Adds a new developer-guide documentation page describing OpenRemote’s CI/CD pipeline concepts introduced during the pipeline migration referenced in openremote/openremote#2696, addressing the documentation request in Issue #161.
Changes:
- Introduces a new
CI/CD Pipelinedeveloper-guide page covering pipeline modification/testing, workflow structure, artifacts, Gradle vs workflow responsibilities, and best/worst practices. - Documents optimization strategies (detect-changes, minimal dependency installs, Git LFS usage, artifact reuse).
- Summarizes GitHub Actions concepts relevant to the pipeline (composite actions, reusable workflows, conditional expressions).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --- | ||
| id: ci-cd-pipeline | ||
| title: CI/CD Pipeline | ||
| sidebar_label: CI/CD Pipeline | ||
| --- |
| ## Deployments | ||
|
|
||
| Deployments are handled by the `deploy.yml` dispatch workflow. This workflow can be triggered manually or via specific staging outcomes. For deployments to test servers, the setup has been minimized to reduce the deployment time significantly. |
| We use a "detect changes" job early in the workflow to determine which files have been modified. Subsequent jobs use the outputs of this job to conditionally skip themselves. This is why UI tests don't run if only backend Java code was touched, significantly speeding up the pipeline. | ||
|
|
||
| ### Minimal Dependency Installation | ||
| Jobs should only install what they need. For Node/Yarn-based projects, instead of running a full `yarn install` everywhere, we use focused installs, e.g., `yarn focus @openremote/util @openremote/test`. This installs the bare minimum dependencies for the scoped workspace, drastically reducing setup time. |
| ### Special Conditions | ||
| You will often see special conditionals used in the pipeline: | ||
| - `if: always()`: Ensures a step runs regardless of whether previous steps failed. This is typically used to ensure test results and coverage reports are uploaded even when tests fail. | ||
| - `if: failure()`: Used to clean up environments or post error notifications only when a job crashes. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ef00fa8ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| We utilize several strategies to ensure the pipeline is as fast and efficient as possible: | ||
|
|
||
| ### Detect Changes | ||
| We use a "detect changes" job early in the workflow to determine which files have been modified. Subsequent jobs use the outputs of this job to conditionally skip themselves. This is why UI tests don't run if only backend Java code was touched, significantly speeding up the pipeline. |
There was a problem hiding this comment.
This example is misleading for backend-only PRs: in the current review.yml and staging.yml workflows, the ui-app-tests reusable workflow is skipped only when needs.detect-changes.outputs.any_modified != 'true', and any_modified includes backend changes. So a Java-only backend change still runs the UI app tests; only the UI-component-specific job is gated on ui_any_modified.
Useful? React with 👍 / 👎.
| We use a "detect changes" job early in the workflow to determine which files have been modified. Subsequent jobs use the outputs of this job to conditionally skip themselves. This is why UI tests don't run if only backend Java code was touched, significantly speeding up the pipeline. | ||
|
|
||
| ### Minimal Dependency Installation | ||
| Jobs should only install what they need. For Node/Yarn-based projects, instead of running a full `yarn install` everywhere, we use focused installs, e.g., `yarn focus @openremote/util @openremote/test`. This installs the bare minimum dependencies for the scoped workspace, drastically reducing setup time. |
There was a problem hiding this comment.
The OpenRemote root package.json declares Yarn 4, and Yarn's documented focus usage is yarn workspaces focus ..., not yarn focus .... If a maintainer copies this example into one of the Node/Yarn jobs, the install step will fail before dependencies are focused, so the example should use yarn workspaces focus @openremote/util @openremote/test.
Useful? React with 👍 / 👎.
|
|
||
| ## Deployments | ||
|
|
||
| Deployments are handled by the `deploy.yml` dispatch workflow. This workflow can be triggered manually or via specific staging outcomes. For deployments to test servers, the setup has been minimized to reduce the deployment time significantly. |
There was a problem hiding this comment.
This sends maintainers to deploy.yml for staging-driven deployments, but the current staging.yml deployment job calls ./.github/workflows/deployment.yml; deploy.yml is the manual/test-server wrapper and is not invoked by staging. When someone changes or debugs deployment behavior after a staging run, following this doc points them at the wrong workflow path.
Useful? React with 👍 / 👎.
|
|
||
| ### Good Practices | ||
| - **Timeout restrictions:** Always add `timeout-minutes: 20` (or an appropriate limit) to any step or job that could potentially hang (such as tests or network-dependent operations). This prevents stalled runners from consuming pipeline minutes indefinitely. | ||
| - **Explicit Secret Passing:** When calling reusable workflows, explicitly pass required secrets using `secrets: { MY_SECRET: ${{ secrets.MY_SECRET }} }`. Alternatively, use `secrets: inherit` if you trust the downstream workflow. |
There was a problem hiding this comment.
The inline flow-map example is not valid YAML as written because the unquoted ${{ ... }} expression contains braces inside another { ... } mapping. If a maintainer copies this secrets: form into a reusable-workflow call, GitHub rejects the workflow file before it runs; use the block mapping style used by the existing workflows or quote the expression.
Useful? React with 👍 / 👎.
|
|
||
| **The most important rule for artifacts is to split them where possible and create artifacts containing only the bare necessities for each specific job.** | ||
|
|
||
| - **What to include:** Compiled distributions, test reports, and coverage files. Do not upload `node_modules` or full `build` folders; compress and selectively upload only the required distributions. |
There was a problem hiding this comment.
This blanket rule conflicts with the current pipeline: install-dist.yml intentionally uploads manager/build, and downstream image/test jobs download that artifact back into manager/build. If maintainers follow this guidance literally and stop uploading full build directories, they can remove artifacts the reusable jobs still require, so the rule should call out the current exception or define what subpaths are safe to trim.
Useful? React with 👍 / 👎.
Description