Skip to content
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

feat(workflows): Add internal package dependencies to existing workflows #592

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

muntaxir4
Copy link
Contributor

@muntaxir4 muntaxir4 commented Dec 17, 2024

User description

Description

We encountered unexpected issues when internal dependencies in the apps were changed. Updating the existing workflows will help catch such errors early and prevent them in the future.

Future Improvements

The paths are currently optimized for amount of workflows running. Example: a change in schema will trigger api-client
only and not workflows for platform and cli.

Developer's checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-check on my work

PR Type

Enhancement


Description

  • Updated GitHub workflow configurations to include internal package dependencies in trigger paths
  • Added schema package as dependency for api-client workflow
  • Added api-client and secret-scan packages as dependencies for CLI workflow
  • Added api-client package as dependency for platform workflow
  • Changes ensure workflows run when dependent packages are modified, helping catch integration issues early

Changes walkthrough 📝

Relevant files
Configuration changes
validate-api-client.yaml
Add schema package dependency to API client workflow         

.github/workflows/validate-api-client.yaml

  • Added packages/schema/** to the paths that trigger workflow
  • Updated both push and pull_request path configurations
  • +10/-2   
    validate-cli.yaml
    Add API client and secret scan dependencies to CLI workflow

    .github/workflows/validate-cli.yaml

  • Added packages/api-client/** and packages/secret-scan/** to trigger
    paths
  • Updated both push and pull_request path configurations
  • +4/-0     
    validate-platform.yaml
    Add API client dependency to platform workflow                     

    .github/workflows/validate-platform.yaml

  • Added packages/api-client/** to workflow trigger paths
  • Updated both push and pull_request path configurations
  • +3/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    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

    Configuration Validation
    Verify that the schema package is indeed a direct dependency of api-client and changes in schema should trigger api-client validation

    Dependencies Check
    Confirm that both api-client and secret-scan packages are direct dependencies of the CLI app and their changes should trigger CLI validation

    Workflow Command
    The lint command is being re-added with a '+' but appears identical to what was removed. Verify if this change is intentional or a merge artifact

    Copy link
    Contributor

    codiumai-pr-agent-free bot commented Dec 17, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    ✅ Fix workflow filename extension mismatch to ensure proper workflow triggering
    Suggestion Impact:Changed file extensions from .yml to .yaml in paths configuration for both workflow files in both push and pull_request triggers

    code diff:

    -        '.github/workflows/validate-platform.yml',
    -        '.github/workflows/deploy-platform.yml'
    +        '.github/workflows/validate-platform.yaml',
    +        '.github/workflows/deploy-platform.yaml'
           ]
       pull_request:
         paths:
           [
             'packages/api-client/**',
             'apps/platform/**',
    -        '.github/workflows/deploy-platform.yml',
    -        '.github/workflows/validate-platform.yml'
    +        '.github/workflows/deploy-platform.yaml',
    +        '.github/workflows/validate-platform.yaml'

    The workflow filename referenced in paths differs from actual filename - using .yml
    extension in paths but actual file has .yaml extension. This mismatch will cause the
    workflow not to trigger properly.

    .github/workflows/validate-platform.yaml [9-14]

     paths:
       [
         'packages/api-client/**',
         'apps/platform/**',
    -    '.github/workflows/validate-platform.yml',
    -    '.github/workflows/deploy-platform.yml'
    +    '.github/workflows/validate-platform.yaml',
    +    '.github/workflows/deploy-platform.yaml'
       ]
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The file extension mismatch (.yml vs .yaml) in the paths configuration could prevent the workflow from triggering correctly, as GitHub Actions uses exact path matching. This is a critical issue that needs to be fixed.

    9
    General
    Standardize YAML array syntax to improve readability and prevent potential parsing errors

    Use consistent YAML array syntax across the workflow file. Currently mixing inline
    array syntax with multi-line block syntax which can lead to parsing issues.
    Standardize on block syntax for better readability and maintainability.

    .github/workflows/validate-api-client.yaml [9-13]

     paths:
    -  [
    -    'packages/schema/**',
    -    'packages/api-client/**',
    -    '.github/workflows/validate-api-client.yml'
    -  ]
    +  - 'packages/schema/**'
    +  - 'packages/api-client/**'
    +  - '.github/workflows/validate-api-client.yml'
    • Apply this suggestion
    Suggestion importance[1-10]: 4

    Why: While both array syntaxes are valid YAML, using dash-based block syntax is more readable and conventional in GitHub Actions workflows. However, this is primarily a style improvement with minimal functional impact.

    4

    Copy link
    Member

    @rajdip-b rajdip-b left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Nice catch dude <3

    @rajdip-b rajdip-b merged commit a9fc39e into keyshade-xyz:develop Dec 19, 2024
    10 checks passed
    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