Edited deploy to staging and readme for website2 #2243
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request primarily enhance the deployment workflow for frontend applications by introducing a new job for Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
CodeRabbit Configuration File (
|
|
New Website2 changes available for preview here |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
.github/workflows/deploy-frontends-to-staging.yml (3)
343-343: Fix indentation in Docker commandsThe Docker tag and push commands are not properly aligned with the build command.
Apply this diff to fix the indentation:
- docker tag ${{ env.REGISTRY_URL }}/${{ secrets.FRONTEND_PROJECT_ID }}/airqo-stage-website2:${{ needs.image-tag.outputs.build_id }} ${{ env.REGISTRY_URL }}/${{ secrets.FRONTEND_PROJECT_ID }}/airqo-stage-website2:latest - docker push ${{ env.REGISTRY_URL }}/${{ secrets.FRONTEND_PROJECT_ID }}/airqo-stage-website2:latest + docker tag ${{ env.REGISTRY_URL }}/${{ secrets.FRONTEND_PROJECT_ID }}/airqo-stage-website2:${{ needs.image-tag.outputs.build_id }} ${{ env.REGISTRY_URL }}/${{ secrets.FRONTEND_PROJECT_ID }}/airqo-stage-website2:latest + docker push ${{ env.REGISTRY_URL }}/${{ secrets.FRONTEND_PROJECT_ID }}/airqo-stage-website2:latest
Line range hint
319-334: Consider consolidating environment variablesThe environment setup for website2 shares several variables with the website job. Consider creating a reusable composite action or workflow template to reduce duplication.
Create a new composite action in
.github/actions/setup-website-envthat can be shared between both website deployments.
Line range hint
360-367: Improve version cleanup logicThe version cleanup script could be enhanced to handle errors and provide better feedback.
Consider this improved version:
- - name: Delete Old Versions - run: |- - service="staging" - versions=$(gcloud app versions list --service=$service --sort-by '~LAST_DEPLOYED' --format 'value(VERSION.ID)' | grep -v 'maintenance' | sort -r | tail -n +4) - for version in $versions; do - echo "Deleting version: $version for service: $service" - gcloud app versions delete "$version" --service=$service --quiet - done + - name: Delete Old Versions + run: |- + service="staging" + echo "Fetching versions for service: $service" + versions=$(gcloud app versions list --service=$service --sort-by '~LAST_DEPLOYED' --format 'value(VERSION.ID)' | grep -v 'maintenance' | sort -r | tail -n +4) || exit 1 + if [ -z "$versions" ]; then + echo "No old versions to clean up" + exit 0 + fi + for version in $versions; do + echo "Deleting version: $version for service: $service" + if ! gcloud app versions delete "$version" --service=$service --quiet; then + echo "Failed to delete version: $version" + exit 1 + fi + done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/deploy-frontends-to-staging.yml(1 hunks)website2/README.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- website2/README.md
🔇 Additional comments (2)
.github/workflows/deploy-frontends-to-staging.yml (2)
Line range hint 29-29: LGTM: Check job modifications for website2
The changes to the check job correctly implement the website2 detection logic, maintaining consistency with other frontend applications.
Also applies to: 52-61
Line range hint 341-359: Verify service name conflicts in App Engine
Both website and website2 are using the same service name "staging" in App Engine. This could lead to deployment conflicts.
Consider using a unique service name for website2, for example:
-service: staging
+service: staging-website2
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit
New Features
website2.website2.Bug Fixes
website2to ensure proper versioning.Documentation
website2/README.mdfor improved clarity.