Skip to content

Conversation

@MaitreyaBuddha
Copy link
Contributor

@MaitreyaBuddha MaitreyaBuddha commented Apr 20, 2025

PR Type

Enhancement


Description

  • Migrate Firebase deploy steps to shared action

  • Remove manual Node, Python, gcloud and CLI setup

  • Pass environment, service_account_json, project_id inputs

  • Add contents read permission for PR title linter


Changes walkthrough 📝

Relevant files
Configuration changes
deploy-functions.yml
Migrate deploy-functions to reusable action                           

.github/workflows/deploy-functions.yml

  • Replaced multiple setup steps with reusable local action
  • Removed Node.js, Python, gcloud and Firebase CLI installs
  • Simplified authentication into with-action inputs
  • Passed environment, service_account_json, project_id
  • +5/-84   
    lint-pr-title.yml
    Grant contents read for PR title lint                                       

    .github/workflows/lint-pr-title.yml

    • Added contents: read permission for workflows
    +1/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    YAML Syntax

    The with: block for the "Deploy Firebase Functions" step isn't marked as added and may be misaligned, resulting in invalid YAML. Ensure it has the correct + prefix and indentation under the uses key.

    - name: Deploy Firebase Functions
      uses: ./
      with:
        environment: ${{ github.event.inputs.environment }}
        service_account_json: ${{ github.event.inputs.environment == 'prod' && secrets.FIREBASE_PROD_SERVICE_ACCOUNT || secrets.FIREBASE_STAGING_SERVICE_ACCOUNT }}
        project_id: ${{ github.event.inputs.environment == 'prod' && 'hello-wisdom-prod' || 'hello-wisdom-staging' }}
    Expression Logic

    The conditional expressions for service_account_json and project_id use &&/||. Verify that these follow GitHub Actions expression rules and yield the correct secrets and project IDs in all environments.

    environment: ${{ github.event.inputs.environment }}
    service_account_json: ${{ github.event.inputs.environment == 'prod' && secrets.FIREBASE_PROD_SERVICE_ACCOUNT || secrets.FIREBASE_STAGING_SERVICE_ACCOUNT }}
    project_id: ${{ github.event.inputs.environment == 'prod' && 'hello-wisdom-prod' || 'hello-wisdom-staging' }}

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Add missing version input

    Include the version input for the composite action if it is required by the action
    metadata.

    .github/workflows/deploy-functions.yml [45-48]

     with:
    +  version: "0.6.4"
       environment: ${{ github.event.inputs.environment }}
       service_account_json: ${{ github.event.inputs.environment == 'prod' && secrets.FIREBASE_PROD_SERVICE_ACCOUNT || secrets.FIREBASE_STAGING_SERVICE_ACCOUNT }}
       project_id: ${{ github.event.inputs.environment == 'prod' && 'hello-wisdom-prod' || 'hello-wisdom-staging' }}
    Suggestion importance[1-10]: 5

    __

    Why: Adding a 'version' input may be helpful if the composite action requires it, and the improved_code correctly shows how to include it; however, the requirement is speculative without examining the action metadata.

    Low

    @MaitreyaBuddha MaitreyaBuddha changed the title Kelly/use action for local deploy2 feat: Use action for local deploy Apr 20, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants