[#2270] Fixed follow-up deployments to Lagoon mark deployment in GitHub as inactive.#2299
Conversation
WalkthroughThis PR updates the default behavior for GitHub deployment notifications to use unique per-PR/branch environment identifiers instead of a generic "PR" value, preventing cross-PR deployment interference. The change affects documentation, the main notify-github.sh script, unit tests, and manual test references. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/docs/content/development/variables.mdx:
- Line 327: Update the "Default value" column for
VORTEX_NOTIFY_GITHUB_ENVIRONMENT_TYPE to reflect the new behavior: show that it
defaults to VORTEX_NOTIFY_LABEL with a fallback of "PR" (e.g.,
"VORTEX_NOTIFY_LABEL (fallback: PR)") instead of just "PR"; update the table
cell text and any inline example references to match the description that the
environment resolves from VORTEX_NOTIFY_LABEL or falls back to PR, and ensure
consistency with the referenced script name notify-github.sh and the variable
VORTEX_NOTIFY_LABEL used in the description.
| | <a id="vortex_notify_event"></a>`VORTEX_NOTIFY_EVENT` | Notification event type.<br/><br/>Can be 'pre_deployment' or 'post_deployment'. | `post_deployment` | `scripts/vortex/notify.sh` | | ||
| | <a id="vortex_notify_github_branch"></a>`VORTEX_NOTIFY_GITHUB_BRANCH` | GitHub notification git branch name. This will be used as the 'ref' parameter in GitHub's Deployment API. | `${VORTEX_NOTIFY_BRANCH}` | `scripts/vortex/notify-github.sh` | | ||
| | <a id="vortex_notify_github_environment_type"></a>`VORTEX_NOTIFY_GITHUB_ENVIRONMENT_TYPE` | GitHub notification environment type: production, uat, dev, pr. Used as the 'environment' parameter in GitHub's Deployment API. | `PR` | `scripts/vortex/notify-github.sh` | | ||
| | <a id="vortex_notify_github_environment_type"></a>`VORTEX_NOTIFY_GITHUB_ENVIRONMENT_TYPE` | GitHub notification environment type. Used as the 'environment' parameter in GitHub's Deployment API. Defaults to VORTEX_NOTIFY_LABEL (e.g. "PR-`123`" or branch name) for unique per-PR/branch environments. This prevents cross-PR deployment interference where deploying one PR would mark another PR's deployment as inactive. | `PR` | `scripts/vortex/notify-github.sh` | |
There was a problem hiding this comment.
Align the “Default value” column with the new default behavior.
The description now says the default derives from VORTEX_NOTIFY_LABEL with a PR fallback, but the default value column still lists PR only. This is internally inconsistent and can mislead users.
📝 Suggested doc fix
-| <a id="vortex_notify_github_environment_type"></a>`VORTEX_NOTIFY_GITHUB_ENVIRONMENT_TYPE` | GitHub notification environment type. Used as the 'environment' parameter in GitHub's Deployment API. Defaults to VORTEX_NOTIFY_LABEL (e.g. "PR-`123`" or branch name) for unique per-PR/branch environments. This prevents cross-PR deployment interference where deploying one PR would mark another PR's deployment as inactive. | `PR` | `scripts/vortex/notify-github.sh` |
+| <a id="vortex_notify_github_environment_type"></a>`VORTEX_NOTIFY_GITHUB_ENVIRONMENT_TYPE` | GitHub notification environment type. Used as the 'environment' parameter in GitHub's Deployment API. Defaults to VORTEX_NOTIFY_LABEL (e.g. "PR-`123`" or branch name) for unique per-PR/branch environments. This prevents cross-PR deployment interference where deploying one PR would mark another PR's deployment as inactive. | `${VORTEX_NOTIFY_LABEL}` (fallback `PR`) | `scripts/vortex/notify-github.sh` |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | <a id="vortex_notify_github_environment_type"></a>`VORTEX_NOTIFY_GITHUB_ENVIRONMENT_TYPE` | GitHub notification environment type. Used as the 'environment' parameter in GitHub's Deployment API. Defaults to VORTEX_NOTIFY_LABEL (e.g. "PR-`123`" or branch name) for unique per-PR/branch environments. This prevents cross-PR deployment interference where deploying one PR would mark another PR's deployment as inactive. | `PR` | `scripts/vortex/notify-github.sh` | | |
| | <a id="vortex_notify_github_environment_type"></a>`VORTEX_NOTIFY_GITHUB_ENVIRONMENT_TYPE` | GitHub notification environment type. Used as the 'environment' parameter in GitHub's Deployment API. Defaults to VORTEX_NOTIFY_LABEL (e.g. "PR-`123`" or branch name) for unique per-PR/branch environments. This prevents cross-PR deployment interference where deploying one PR would mark another PR's deployment as inactive. | `${VORTEX_NOTIFY_LABEL}` (fallback `PR`) | `scripts/vortex/notify-github.sh` | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/docs/content/development/variables.mdx at line 327, Update the
"Default value" column for VORTEX_NOTIFY_GITHUB_ENVIRONMENT_TYPE to reflect the
new behavior: show that it defaults to VORTEX_NOTIFY_LABEL with a fallback of
"PR" (e.g., "VORTEX_NOTIFY_LABEL (fallback: PR)") instead of just "PR"; update
the table cell text and any inline example references to match the description
that the environment resolves from VORTEX_NOTIFY_LABEL or falls back to PR, and
ensure consistency with the referenced script name notify-github.sh and the
variable VORTEX_NOTIFY_LABEL used in the description.
|
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2299 +/- ##
==========================================
- Coverage 77.69% 77.12% -0.58%
==========================================
Files 117 110 -7
Lines 6174 6015 -159
Branches 44 0 -44
==========================================
- Hits 4797 4639 -158
+ Misses 1377 1376 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Closes #2270
Summary by CodeRabbit
Bug Fixes
Documentation