-
-
Notifications
You must be signed in to change notification settings - Fork 28
[#1456] Updated code artefact packaging tool to be sourced from a stable minor version. #2102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughUpdated the git-artifact version constraint from Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2102 +/- ##
========================================
Coverage 70.02% 70.02%
========================================
Files 97 97
Lines 4898 4898
Branches 44 44
========================================
Hits 3430 3430
Misses 1468 1468 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ble minor version.
7d88b90 to
59576c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.vortex/tests/bats/unit/deploy-artifact.bats(1 hunks)scripts/vortex/deploy-artifact.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.
Applied to files:
.vortex/tests/bats/unit/deploy-artifact.bats
📚 Learning: 2025-07-23T01:16:30.963Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1816
File: .docker/cli.dockerfile:72-76
Timestamp: 2025-07-23T01:16:30.963Z
Learning: In the Vortex project, unauthenticated Composer installs should be allowed, so GitHub token secrets should not be marked as `required=true` in Docker build mounts. The conditional check `if [ -s /run/secrets/github_token ]` is the preferred approach to allow builds to proceed without a token when needed.
Applied to files:
scripts/vortex/deploy-artifact.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (1)
.vortex/tests/bats/unit/deploy-artifact.bats (1)
103-103: LGTM! Test expectation updated to match implementation.The test expectation has been correctly updated from
git-artifact:^1togit-artifact:~1.1, maintaining consistency with the implementation change inscripts/vortex/deploy-artifact.shline 82.
|
|
||
| task "Installing artifact builder." | ||
| composer global require --dev -n --ansi --prefer-source --ignore-platform-reqs drevops/git-artifact:^1 | ||
| composer global require --dev -n --ansi --prefer-source --ignore-platform-reqs drevops/git-artifact:~1.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that git-artifact version 1.1.x exists and addresses the upstream issue.
The version constraint change from ^1 to ~1.1 is more restrictive and pins to the 1.1.x minor version. Please confirm that:
- Version 1.1.x of drevops/git-artifact exists and is stable
- It specifically addresses the branch deletion issue mentioned in upstream issue Fixed composer config + updates. #92
- There are no breaking changes or compatibility concerns between 1.0.x and 1.1.x
🏁 Script executed:
#!/bin/bash
# Check available versions of drevops/git-artifact on Packagist
curl -s https://repo.packagist.org/p2/drevops/git-artifact.json | jq -r '.packages["drevops/git-artifact"] | keys[]' | sort -V | tail -10Length of output: 29
🌐 Web query:
Does drevops/git-artifact version 1.1 fix the issue where artifact deployment fails when the source branch no longer exists?
💡 Result:
Short answer: No — there is no drevops/git-artifact 1.1 release, so it cannot have fixed that bug.
Evidence:
- Packagist lists available versions and shows 1.0.0 as the latest release (no 1.1). [1]
- The project's GitHub repository is active (source/ tags) but no 1.1 release is published. [2]
If you want, I can: (a) search the repo issues/pull-requests for that specific "source branch no longer exists" failure and report whether a fix/PR exists, or (b) show how to work around the problem in CI.
Version constraint references non-existent release; composer install will fail.
The version constraint change to ~1.1 references a release that does not exist. Version 1.1 is not available on Packagist, with 1.0.0 being the latest release. The ~1.1 constraint will cause composer to fail during installation.
Update the constraint to target an existing version (e.g., ^1.0 for 1.0.0) or wait for an actual 1.1.x release before merging this change.
🤖 Prompt for AI Agents
In scripts/vortex/deploy-artifact.sh around line 82, the composer version
constraint uses ~1.1 which references a non-existent release and will cause
composer to fail; change the constraint to an existing version such as ^1.0 (or
the exact 1.0.0) so composer can install successfully, e.g., replace ~1.1 with
^1.0 (or pin 1.0.0) before merging.
Closes #1456
Summary by CodeRabbit