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

chore: Verify package version #141

Merged
merged 1 commit into from
Oct 23, 2024
Merged

chore: Verify package version #141

merged 1 commit into from
Oct 23, 2024

Conversation

seeratawan01
Copy link
Member

@seeratawan01 seeratawan01 commented Oct 23, 2024

PR Type

enhancement, configuration changes


Description

  • Updated the CI workflow to change the artifact name from packages-dist to packages-artifacts and included additional files like package.json and README.md in the artifact paths.
  • Reordered steps in the workflow to install pnpm after downloading artifacts.
  • Added a new step to verify the package version by reading the package.json file.
  • Modified the pnpm install command to use the --frozen-lockfile option to ensure consistency.

Changes walkthrough 📝

Relevant files
Configuration changes
cd-develop.yml
Update CI workflow for artifact handling and package verification

.github/workflows/cd-develop.yml

  • Changed artifact name from packages-dist to packages-artifacts.
  • Updated artifact paths to include package.json and README.md.
  • Moved Install pnpm step to occur after downloading artifacts.
  • Added a step to verify the package version by reading package.json.
  • Changed pnpm install to use --frozen-lockfile.
  • +39/-22 

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

    @seeratawan01 seeratawan01 merged commit ad516b6 into develop Oct 23, 2024
    1 of 2 checks passed
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🏅 Score: 85
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Bug
    The 'Verify package version' step only prints the package.json file without verifying the version. This might not perform the intended check.

    Redundancy
    The 'Install pnpm' step is repeated in multiple jobs which could be optimized by defining it once using reusable workflows or a pre-job setup.

    Code feedback:
    relevant file.github/workflows/cd-develop.yml
    suggestion      

    Consider adding error handling or a version check in the 'Verify package version' step to ensure it performs a meaningful check. [important]

    relevant linecat packages/javascript-sdk/package.json

    relevant file.github/workflows/cd-develop.yml
    suggestion      

    To avoid redundancy and ensure consistency, consider using a single step to install 'pnpm' that can be reused across different jobs. This could be achieved by using a pre-job setup or GitHub Actions' reusable workflows feature. [important]

    relevant linerun: npm install -g pnpm

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Add version verification step to ensure the package version is as expected before publishing

    Verify the output of the cat command to ensure the package version matches
    expectations before proceeding to publish.

    .github/workflows/cd-develop.yml [113-115]

     run: |
    -    cat packages/javascript-sdk/package.json
    +    version=$(jq '.version' packages/javascript-sdk/package.json)
    +    if [[ "$version" == "expected_version" ]]; then
    +      echo "Version verified."
    +    else
    +      echo "Version mismatch."
    +      exit 1
    +    fi
    Suggestion importance[1-10]: 8

    Why: The suggestion to verify the package version before publishing is valuable for ensuring that the correct version is being released, which can prevent accidental releases of incorrect versions.

    8
    Best practice
    Add error handling after dependency installation to ensure it completes successfully

    Consider adding error handling or a verification step after installing dependencies
    with pnpm to ensure the installation was successful before proceeding.

    .github/workflows/cd-develop.yml [110]

    -run: pnpm install --frozen-lockfile
    +run: pnpm install --frozen-lockfile && echo "Dependencies installed successfully" || echo "Installation failed"
    Suggestion importance[1-10]: 7

    Why: Adding error handling after installing dependencies with pnpm is a good practice to ensure the installation was successful before proceeding. This can help catch issues early and improve the reliability of the workflow.

    7
    Specify a version range for Node.js to safeguard against breaking changes in new releases

    Use a specific version or range for node-version to avoid potential issues with new
    major releases that might introduce breaking changes.

    .github/workflows/cd-develop.yml [97]

    -node-version: '20'
    +node-version: '^20.0'
    Suggestion importance[1-10]: 6

    Why: Using a version range for Node.js can help prevent issues caused by breaking changes in new major releases, enhancing the stability of the workflow. However, the impact is moderate as it depends on the project's specific requirements.

    6

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant