-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
[workflows] Fix preview deployment inactivation #843
Conversation
📝 WalkthroughWalkthroughThe pull request modifies the GitHub Actions workflow file Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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: 0
🧹 Outside diff range and nitpick comments (3)
.github/workflows/website-preview-destroy.yml (3)
60-66
: Use semantic versioning instead of commit hashThe deployment-status action is pinned to a specific commit hash. While this is secure, it makes it harder to track which version is being used. Consider using the semantic version tag
v2.0.3
instead of9a72af4586197112e0491ea843682b5dc280d806
.- uses: chrnorm/deployment-status@9a72af4586197112e0491ea843682b5dc280d806 # v2.0.3 + uses: chrnorm/[email protected]
51-59
: Consider making the status parameter dynamicThe
status: success
parameter is hardcoded. Consider making this dynamic based on the actual state of the deployment or previous steps.- status: success + status: ${{ inputs.deployment_status || 'success' }}
51-66
: Overall implementation looks good! 💪The new deployment management steps effectively handle the PR objective of managing deployments per PR. The two-step process (seek and inactivate) provides a clean way to manage deployment states.
Consider adding error handling and logging to help debug deployment issues in production:
- Add step outputs for deployment status
- Include conditional steps for handling failed deployment lookups
- Add logging for important state changes
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/website-preview-destroy.yml
(1 hunks)
🔇 Additional comments (2)
.github/workflows/website-preview-destroy.yml (2)
51-59
: Consider using a more stable version of the seek-deployment action
The cloudposse/[email protected]
is an early version (0.x.x), which might be unstable. Consider:
- Verifying if a newer, more stable version is available
- Adding a comment explaining why this specific version was chosen if it's intentional
✅ Verification successful
The action version is appropriate and actively maintained
The cloudposse/[email protected]
is actually the latest version and the repository shows active maintenance with the most recent commit in September 2024. The version numbering, while early, appears intentional as the action maintains a focused, stable functionality. No open issues indicate stability concerns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for newer versions of the action
gh api repos/cloudposse/github-action-seek-deployment/releases --jq '.[].tag_name'
Length of output: 96
Script:
#!/bin/bash
# Let's check the action's repository for any issues or discussions about version stability
gh api repos/cloudposse/github-action-seek-deployment/issues --jq '.[] | select(.state=="open") | {title: .title, url: .html_url}'
# Also check the latest commit date to see if the project is actively maintained
gh api repos/cloudposse/github-action-seek-deployment/commits --jq '.[0] | {sha: .sha, date: .commit.author.date}'
Length of output: 330
51-59
: Validate branch name before using it as deployment ref
The ref
parameter uses the extracted branch name directly without validation. Consider adding input validation to ensure the branch name meets GitHub's requirements.
@goruha I don't understand the description (please GPT!) Just want to be clear, we are working on MANY parallel PRs and want previews for all of them, concurrently. |
@osterman exactly |
These changes were released in v1.123.0. |
what
why
Summary by CodeRabbit