Skip to content

feat: add CI for auto publishing CLI #10

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.

Sign up for GitHub

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

Merged
aimensahnoun merged 1 commit into from
Oct 4, 2024
Merged

feat: add CI for auto publishing CLI #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .github/workflow/npm-publish.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: release-cli-if-necessary

on:
push:
branches:
- main
workflow_dispatch:

jobs:
release-cli-if-necessary:
runs-on: ubuntu-latest
steps:
- name: Checkout repository 🛎️
uses: actions/checkout@v4

- name: Set up Node.js
uses: actions/setup-node@v3
with:
node-version: '18'
registry-url: 'https://registry.npmjs.org'

- name: Install dependencies 📥
run: npm ci
Comment on lines +9 to +23
Copy link

@coderabbitai coderabbitai bot Oct 4, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Action Required: Node.js version not specified in project

The workflow specifies Node.js v18, but the project does not specify which Node.js version to use. To ensure consistency and clarity, please specify the Node.js version in either:

  • The engines field of package.json, for example: 
    "engines": {
  "node": ">=18"
}
  • A .nvmrc file at the root of the project with the desired Node.js version, e.g., 18.
🔗 Analysis chain

LGTM: Job setup and initial steps are well-configured.

The job configuration and initial steps are appropriate for a Node.js project:

  • Using the latest Ubuntu ensures good security and feature support.
  • The repository checkout and Node.js setup are correctly implemented.
  • Using npm ci for installing dependencies is the right choice for CI environments.

Please verify that Node.js v18 is the intended version for this project. If a different version is required, update the node-version field in the Node.js setup step.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the Node.js version specified in the project

# Test: Check for Node.js version specification in package.json or .nvmrc
rg --type json '"node":' package.json
cat .nvmrc 2>/dev/null || echo "No .nvmrc file found"

Length of output: 265

Script:

#!/bin/bash
# Description: Verify the Node.js version specified in the project

# Check for Node.js version in package.json's "engines" field
rg '"engines"\s*:\s*{\s*"node"\s*:\s*"' package.json

# Check for .nvmrc file and display its content if it exists
if [ -f .nvmrc ]; then
  cat .nvmrc
else
  echo "No .nvmrc file found."
fi

Length of output: 261

Script:

#!/bin/bash
# Description: Verify the Node.js version specified in the project

# Check for Node.js version in package.json's "engines" field using jq
if command -v jq >/dev/null 2>&1; then
  node_version=$(jq -r '.engines.node // empty' package.json)
  if [ -n "$node_version" ]; then
    echo "Node.js version specified in package.json: $node_version"
  else
    echo "No Node.js version specified in package.json."
  fi
else
  echo "jq is not installed. Cannot parse package.json."
fi

# Check for .nvmrc file and display its content if it exists
if [ -f .nvmrc ]; then
  echo "Node.js version specified in .nvmrc:"
  cat .nvmrc
else
  echo "No .nvmrc file found."
fi

Length of output: 268


- name: Make is-release-needed.sh executable
run: chmod +x ./scripts/is-release-needed.sh

- name: Check if version number has already been released 🕵️‍♀️
id: is-release-needed
run: |
echo "Checking if request-injector is already published..."
IS_RELEASE_NEEDED=$(./scripts/is-release-needed.sh)
echo "is-release-needed=$IS_RELEASE_NEEDED"
echo "is-release-needed=$IS_RELEASE_NEEDED" >> $GITHUB_OUTPUT

- name: Build package
if: steps.is-release-needed.outputs.is-release-needed == 'true'
run: npm run build

- name: Publish package on NPM 📦
if: steps.is-release-needed.outputs.is-release-needed == 'true'
run: npm publish
env:
NODE_AUTH_TOKEN: ${{ secrets.REQUEST_BOT_NPM_TOKEN }}
13 changes: 13 additions & 0 deletions scripts/is-release-needed.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/usr/bin/env bash

# This script checks if the current version of the package is already published on npm
PACKAGE_VERSION="$(node -p -e "require('./package.json').version")"
PACKAGE_NAME="$(node -p -e "require('./package.json').name")"
Comment on lines +1 to +5
Copy link

@coderabbitai coderabbitai bot Oct 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve robustness of package information extraction

The script correctly uses the shebang for Bash and extracts package information using Node.js. However, to enhance robustness:

  1. Consider adding error handling for cases where Node.js is not installed or package.json is not found.
  2. Quote the Node.js commands to handle potential spaces or special characters in package names or versions.

Here's an improved version:

 #!/usr/bin/env bash

 # This script checks if the current version of the package is already published on npm
-PACKAGE_VERSION="$(node -p -e "require('./package.json').version")"
-PACKAGE_NAME="$(node -p -e "require('./package.json').name")"
+if ! command -v node &> /dev/null; then
+    echo "Error: Node.js is required but not installed." >&2
+    exit 1
+fi
+
+if [ ! -f "./package.json" ]; then
+    echo "Error: package.json not found in the current directory." >&2
+    exit 1
+fi
+
+PACKAGE_VERSION="$(node -p -e "require('./package.json').version")" || { echo "Error: Failed to extract package version." >&2; exit 1; }
+PACKAGE_NAME="$(node -p -e "require('./package.json').name")" || { echo "Error: Failed to extract package name." >&2; exit 1; }

This change adds error handling and ensures the script fails gracefully if prerequisites are not met.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#!/usr/bin/env bash
# This script checks if the current version of the package is already published on npm
PACKAGE_VERSION="$(node -p -e "require('./package.json').version")"
PACKAGE_NAME="$(node -p -e "require('./package.json').name")"
#!/usr/bin/env bash
# This script checks if the current version of the package is already published on npm
if ! command -v node &> /dev/null; then
echo "Error: Node.js is required but not installed." >&2
exit 1
fi
if [ ! -f "./package.json" ]; then
echo "Error: package.json not found in the current directory." >&2
exit 1
fi
PACKAGE_VERSION="$(node -p -e "require('./package.json').version")" || { echo "Error: Failed to extract package version." >&2; exit 1; }
PACKAGE_NAME="$(node -p -e "require('./package.json').name")" || { echo "Error: Failed to extract package name." >&2; exit 1; }


FOUND_VERSION=$(npm view $PACKAGE_NAME versions | grep $PACKAGE_VERSION)
Copy link

@coderabbitai coderabbitai bot Oct 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance npm version check reliability

The current implementation checks for the package version on npm, but it can be improved for better reliability and to address potential issues flagged by Shellcheck.

Consider the following improvements:

-FOUND_VERSION=$(npm view $PACKAGE_NAME versions | grep $PACKAGE_VERSION)
+FOUND_VERSION=$(npm view "$PACKAGE_NAME" versions --json 2>/dev/null | grep -q "\"$PACKAGE_VERSION\"" && echo "found" || echo "")

This change:

  1. Adds double quotes to prevent globbing and word splitting, as suggested by Shellcheck.
  2. Uses the --json flag for more reliable parsing.
  3. Redirects stderr to /dev/null to suppress potential warnings.
  4. Uses grep with -q for a quiet check and sets FOUND_VERSION based on the grep result.

These modifications make the version check more robust and less prone to errors from unexpected npm output or special characters in the package name or version.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
FOUND_VERSION=$(npm view $PACKAGE_NAME versions | grep $PACKAGE_VERSION)
FOUND_VERSION=$(npm view "$PACKAGE_NAME" versions --json 2>/dev/null | grep -q "\"$PACKAGE_VERSION\"" && echo "found" || echo "")
🧰 Tools
🪛 Shellcheck

[info] 7-7: Double quote to prevent globbing and word splitting.

(SC2086)

[info] 7-7: Double quote to prevent globbing and word splitting.

(SC2086)


if [ -z "$FOUND_VERSION" ]; then
echo 'true' # release needed
else
echo 'false' # release not needed
fi
Comment on lines +1 to +13
Copy link

@coderabbitai coderabbitai bot Oct 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance overall script structure and error handling

While the individual parts of the script have been addressed in previous comments, there's an opportunity to improve the overall structure and error handling of the script.

Consider the following improvements:

  1. Add a main function to encapsulate the script logic.
  2. Implement proper error handling and logging.
  3. Use set options to make the script more robust.

Here's a suggested restructure of the entire script:

#!/usr/bin/env bash

set -euo pipefail

log_error() {
    echo "ERROR: $*" >&2
}

check_prerequisites() {
    if ! command -v node &> /dev/null; then
        log_error "Node.js is required but not installed."
        exit 1
    fi

    if [ ! -f "./package.json" ]; then
        log_error "package.json not found in the current directory."
        exit 1
    fi
}

get_package_info() {
    PACKAGE_VERSION=$(node -p -e "require('./package.json').version") || { log_error "Failed to extract package version."; exit 1; }
    PACKAGE_NAME=$(node -p -e "require('./package.json').name") || { log_error "Failed to extract package name."; exit 1; }
}

check_npm_version() {
    if npm view "$PACKAGE_NAME" versions --json 2>/dev/null | grep -q "\"$PACKAGE_VERSION\""; then
        echo "Release not needed: Version $PACKAGE_VERSION of $PACKAGE_NAME already exists on npm"
        exit 1
    else
        echo "Release needed: Version $PACKAGE_VERSION of $PACKAGE_NAME not found on npm"
        exit 0
    fi
}

main() {
    check_prerequisites
    get_package_info
    check_npm_version
}

main "$@"

This restructured version:

  1. Uses a main function to organize the script flow.
  2. Implements proper error handling and logging.
  3. Uses set -euo pipefail for safer script execution.
  4. Breaks down the logic into smaller, focused functions.

These changes make the script more robust, easier to maintain, and more aligned with best practices for Bash scripting.

🧰 Tools
🪛 Shellcheck

[info] 7-7: Double quote to prevent globbing and word splitting.

(SC2086)

[info] 7-7: Double quote to prevent globbing and word splitting.

(SC2086)

Comment on lines +9 to +13
Copy link

@coderabbitai coderabbitai bot Oct 4, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider enhancing the output format for better CI/CD integration

The current logic for determining if a release is needed is correct. However, the output format can be improved for better integration with CI/CD pipelines and increased clarity.

Consider the following enhancements:

  1. Use exit codes along with the output for easier script integration.
  2. Provide more informative output messages.

Here's an improved version:

 if [ -z "$FOUND_VERSION" ]; then
-    echo 'true' # release needed
+    echo "Release needed: Version $PACKAGE_VERSION of $PACKAGE_NAME not found on npm"
+    exit 0  # Exit code 0 indicates a release is needed
 else
-    echo 'false' # release not needed
+    echo "Release not needed: Version $PACKAGE_VERSION of $PACKAGE_NAME already exists on npm"
+    exit 1  # Exit code 1 indicates a release is not needed
 fi

This change provides more context in the output and uses standard exit codes, making it easier to integrate this script into CI/CD workflows and interpret its results.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ -z "$FOUND_VERSION" ]; then
echo 'true' # release needed
else
echo 'false' # release not needed
fi
if [ -z "$FOUND_VERSION" ]; then
echo "Release needed: Version $PACKAGE_VERSION of $PACKAGE_NAME not found on npm"
exit 0 # Exit code 0 indicates a release is needed
else
echo "Release not needed: Version $PACKAGE_VERSION of $PACKAGE_NAME already exists on npm"
exit 1 # Exit code 1 indicates a release is not needed
fi