Skip to content

PR standardization #211

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

Open
cb-anomitromunshi wants to merge 5 commits into
base: master
Choose a base branch
from
Open

PR standardization #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d8be9a8
PR standardization
cb-anomitromunshi Mar 12, 2026
c014a1f
PR standardization
cb-anomitromunshi Mar 12, 2026
9ea85e9
PR standardization
cb-anomitromunshi Mar 12, 2026
035afd1
PR standardization
cb-anomitromunshi Mar 12, 2026
eff453f
PR standardization
cb-anomitromunshi Mar 12, 2026
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
29 changes: 29 additions & 0 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
## CHANGELOG
REPLACE_ME_WITH_CHANGELOG

## SUMMARY
REPLACE_ME_WITH_SUMMARY_OF_THE_CHANGES

## FUNCTIONAL AUTOMATION CHANGES PR
- [ ] Yes
- If Yes, PR :
- [ ] No
- If No, Reason:

## AUTOMATION TEST REPORT URL
REPLACE_ME_WITH_TEST_REPORT_URL

## AREAS OF IMPACT
REPLACE_ME_WITH_AREAS_OF_IMPACT_OR_NA

## TYPE OF CHANGE
- [ ] 🐞 Bugfix
- [ ] 🌟 Feature
- [ ] ✨ Enhancement
- [ ] 🧪 Unit Test Cases
- [ ] 📔 Documentation
- [ ] ⚙️ Chore - Build Related / Configuration / Others


## DOCUMENTATION
REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA
Comment on lines +28 to +29
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟢 LOW

Problem: The file is missing a trailing newline (\n), as indicated by \ No newline at end of file.

Why: Many tools and linters expect text files to end with a newline; missing it can cause unnecessary diffs, minor tooling issues, or violate common repository style conventions.

How to Fix: Add a newline at the end of the file so the last line is properly terminated.

Suggested change
## DOCUMENTATION
REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA
## DOCUMENTATION
REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA

31 changes: 31 additions & 0 deletions .github/scripts/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Scripts

## Generate changelog

Lists PRs merged into a branch (excludes "Parent branch sync" and bot-authored PRs).

**Prerequisites:** `jq`, and GitHub credentials as env vars.

### Set credentials (once per terminal)

```bash
export GH_USERNAME=your-github-username
export GH_PAT=your-github-personal-access-token
```

### Commands

From repo root:

| What you want | Command |
|---------------|---------|
| PRs merged into **current branch** (last 30 days) | `./.github/scripts/generate-changelog.sh` |
| PRs merged into **master** (last 30 days) | `./.github/scripts/generate-changelog.sh master` |
| PRs merged into **master** since a date | `./.github/scripts/generate-changelog.sh master "merged:>=2025-01-01"` |

### Arguments

1. **Branch** (optional) — default: current branch
2. **Date filter** (optional) — default: last 30 days (e.g. `merged:>=2025-01-01`)

Output includes a GitHub search URL to verify results.
84 changes: 84 additions & 0 deletions .github/scripts/generate-changelog.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
#!/bin/bash

set -e

# Check required environment variables
if [[ -z "$GH_USERNAME" ]]; then
echo "❌ Error: Environment variable GH_USERNAME not found"
exit 1
fi

if [[ -z "$GH_PAT" ]]; then
echo "❌ Error: Environment variable GH_PAT not found"
exit 1
fi

# Optional: Branch name (defaults to current branch if not provided)
SOURCE_BRANCH="${1:-$(git branch --show-current)}"
# Optional: Date filter (defaults to last 30 days if not provided)
DATE_FILTER="${2:-merged:>=$(date -u -v-30d +%Y-%m-%d 2>/dev/null || date -u -d '30 days ago' +%Y-%m-%d)}"

# Repo is set per-repo when this file is pushed (placeholder replaced by upload script)
REPO="chargebee/chargebee-react-native"

echo "🔍 Searching for PRs merged into $SOURCE_BRANCH..."
Comment on lines +18 to +24
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: The REPO variable is hardcoded to chargebee/chargebee-react-native, which makes this script non-reusable across repositories and contradicts the comment that it will be set per-repo when pushed.

Why: If this script is copied to another repository without updating REPO, it will silently query and generate changelogs for the wrong repo, leading to incorrect release notes and confusion; the misleading comment also increases the risk of misconfiguration.

How to Fix: Either derive REPO dynamically from git remote or require it via an environment variable (with a clear error if missing), and update the comment accordingly so the behavior is explicit and repo-agnostic.

Suggested change
# Optional: Date filter (defaults to last 30 days if not provided)
DATE_FILTER="${2:-merged:>=$(date -u -v-30d +%Y-%m-%d 2>/dev/null || date -u -d '30 days ago' +%Y-%m-%d)}"
# Repo is set per-repo when this file is pushed (placeholder replaced by upload script)
REPO="chargebee/chargebee-react-native"
echo "🔍 Searching for PRs merged into $SOURCE_BRANCH..."
# Optional: Date filter (defaults to last 30 days if not provided)
DATE_FILTER="${2:-merged:>=$(date -u -v-30d +%Y-%m-%d 2>/dev/null || date -u -d '30 days ago' +%Y-%m-%d)}"
# Repository to query (must be provided as OWNER/REPO)
if [[ -z "$REPO" ]]; then
echo "❌ Error: Environment variable REPO (format OWNER/REPO) not found"
exit 1
fi
echo "🔍 Searching for PRs merged into $SOURCE_BRANCH..."


# GitHub API call with error handling
HTTP_STATUS=$(curl -s -w "%{http_code}" -G -u "$GH_USERNAME:$GH_PAT" \
"https://api.github.com/search/issues" \
--data-urlencode "q=NOT \"Parent branch sync\" in:title is:pr repo:$REPO is:merged base:$SOURCE_BRANCH merged:$DATE_FILTER -author:app/distributed-gitflow-app" \
-o /tmp/curl_output.json \
2>/tmp/curl_error.log)

CURL_EXIT_CODE=$?
Comment on lines +27 to +33
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: The script writes API responses and errors to fixed paths under /tmp (/tmp/curl_output.json, /tmp/curl_error.log), which can cause collisions when multiple instances run concurrently and may expose sensitive data in shared temp files.

Why: Concurrent executions (e.g., multiple users or CI jobs on the same runner) can overwrite each other’s temp files, leading to incorrect results or confusing diagnostics; additionally, storing API responses with potentially sensitive metadata in predictable global paths increases the risk of unintended access.

How to Fix: Use uniquely named temporary files (e.g., via mktemp) scoped to the current process, and ensure they are cleaned up at the end or on error.

Suggested change
HTTP_STATUS=$(curl -s -w "%{http_code}" -G -u "$GH_USERNAME:$GH_PAT" \
"https://api.github.com/search/issues" \
--data-urlencode "q=NOT \"Parent branch sync\" in:title is:pr repo:$REPO is:merged base:$SOURCE_BRANCH merged:$DATE_FILTER -author:app/distributed-gitflow-app" \
-o /tmp/curl_output.json \
2>/tmp/curl_error.log)
CURL_EXIT_CODE=$?
OUTPUT_FILE="$(mktemp /tmp/changelog_curl_output.XXXXXX.json)"
ERROR_FILE="$(mktemp /tmp/changelog_curl_error.XXXXXX.log)"
HTTP_STATUS=$(curl -s -w "%{http_code}" -G -u "$GH_USERNAME:$GH_PAT" \
"https://api.github.com/search/issues" \
--data-urlencode "q=NOT \"Parent branch sync\" in:title is:pr repo:$REPO is:merged base:$SOURCE_BRANCH merged:$DATE_FILTER -author:app/distributed-gitflow-app" \
-o "$OUTPUT_FILE" \
2>"$ERROR_FILE")
CURL_EXIT_CODE=$?


echo "🌐 API call status: $HTTP_STATUS"

if [ $CURL_EXIT_CODE -ne 0 ]; then
echo "❌ Error: curl request failed with exit code $CURL_EXIT_CODE"
echo "Error details: $(cat /tmp/curl_error.log)"
exit 1
fi
Comment on lines +37 to +41
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: When curl fails, the script unconditionally runs cat /tmp/curl_error.log, which will itself fail if the file is missing or renamed (e.g., after refactoring to use mktemp), and it doesn’t guard against empty error output.

Why: A secondary failure while printing error details can obscure the original problem and make debugging harder, especially in CI logs where only the last error is visible.

How to Fix: Check for the existence and non-emptiness of the error file before reading it, and fall back to a generic message if it’s not available.

Suggested change
if [ $CURL_EXIT_CODE -ne 0 ]; then
echo "❌ Error: curl request failed with exit code $CURL_EXIT_CODE"
echo "Error details: $(cat /tmp/curl_error.log)"
exit 1
fi
if [ $CURL_EXIT_CODE -ne 0 ]; then
echo "❌ Error: curl request failed with exit code $CURL_EXIT_CODE"
if [[ -s "$ERROR_FILE" ]]; then
echo "Error details: $(cat "$ERROR_FILE")"
else
echo "Error details: (no additional error output available)"
fi
exit 1
fi


if [ "$HTTP_STATUS" -ne 200 ]; then
echo "❌ Error: API returned HTTP status $HTTP_STATUS"
echo "Response: $(cat /tmp/curl_output.json)"
exit 1
fi
Comment on lines +43 to +47
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: On non-200 HTTP responses, the script prints the entire JSON response inline with $(cat /tmp/curl_output.json), which can be very large and hard to read, and again assumes the fixed temp path exists.

Why: Large unformatted JSON blobs reduce log readability and can make it difficult to spot the actual error message; relying on a hardcoded temp path also breaks if the file naming strategy changes.

How to Fix: Use the same temp variable as for the curl output, and either pretty-print the JSON with jq when available or truncate/indicate where to inspect the full response.

Suggested change
if [ "$HTTP_STATUS" -ne 200 ]; then
echo "❌ Error: API returned HTTP status $HTTP_STATUS"
echo "Response: $(cat /tmp/curl_output.json)"
exit 1
fi
if [ "$HTTP_STATUS" -ne 200 ]; then
echo "❌ Error: API returned HTTP status $HTTP_STATUS"
if command -v jq >/dev/null 2>&1; then
echo "Response:"
jq . "$OUTPUT_FILE" || cat "$OUTPUT_FILE"
else
echo "Response: $(cat "$OUTPUT_FILE")"
fi
exit 1
fi


PR_LIST_RESPONSE=$(cat /tmp/curl_output.json)

# Clean invalid control characters from JSON response
Comment on lines +49 to +51
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: The script reads the entire API response into a shell variable (PR_LIST_RESPONSE=$(cat /tmp/curl_output.json)), which is unnecessary and can be fragile for large responses.

Why: Storing large JSON payloads in shell variables can hit shell limits and makes the script more memory-intensive; it’s more robust to stream from the file directly into jq and other tools.

How to Fix: Avoid loading the full JSON into a variable; instead, operate directly on the output file with jq and only assign small scalar values (like total_count) to variables.

Suggested change
PR_LIST_RESPONSE=$(cat /tmp/curl_output.json)
# Clean invalid control characters from JSON response
PR_LIST_RESPONSE_FILE="$OUTPUT_FILE"
# Clean invalid control characters from JSON response

if ! echo "$PR_LIST_RESPONSE" | jq . >/dev/null 2>&1; then
echo "⚠️ Invalid JSON detected — cleaning control characters..."
PR_LIST_RESPONSE=$(echo "$PR_LIST_RESPONSE" | tr -d '\000-\037')

if ! echo "$PR_LIST_RESPONSE" | jq . >/dev/null 2>&1; then
echo "$PR_LIST_RESPONSE" > /tmp/invalid_json_debug.json
echo "❌ Error: JSON is still invalid after cleaning control characters"
echo "💡 Use 'cat /tmp/invalid_json_debug.json' to inspect the JSON"
exit 1
fi
fi
Comment on lines +51 to +62
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: The JSON “cleaning” step uses tr -d '\000-\037', which strips all control characters including legitimate whitespace like newlines and tabs, potentially altering valid JSON content.

Why: Removing all control characters can corrupt otherwise valid JSON (e.g., strings containing tabs or newlines), leading to subtle parsing issues; it’s safer to target only known problematic characters or handle encoding issues differently.

How to Fix: Narrow the cleaning to specific problematic characters (e.g., \000-\010\013\014\016-\037) or, if the issue is rare, fail fast with a clear error and let the user inspect the raw JSON rather than mutating it aggressively.

Suggested change
# Clean invalid control characters from JSON response
if ! echo "$PR_LIST_RESPONSE" | jq . >/dev/null 2>&1; then
echo "⚠️ Invalid JSON detected — cleaning control characters..."
PR_LIST_RESPONSE=$(echo "$PR_LIST_RESPONSE" | tr -d '\000-\037')
if ! echo "$PR_LIST_RESPONSE" | jq . >/dev/null 2>&1; then
echo "$PR_LIST_RESPONSE" > /tmp/invalid_json_debug.json
echo "❌ Error: JSON is still invalid after cleaning control characters"
echo "💡 Use 'cat /tmp/invalid_json_debug.json' to inspect the JSON"
exit 1
fi
fi
# Clean invalid control characters from JSON response
if ! jq . "$PR_LIST_RESPONSE_FILE" >/dev/null 2>&1; then
echo "⚠️ Invalid JSON detected — cleaning control characters..."
CLEANED_FILE="$(mktemp /tmp/changelog_cleaned_json.XXXXXX.json)"
tr -d '\000-\010\013\014\016-\037' < "$PR_LIST_RESPONSE_FILE" > "$CLEANED_FILE"
if ! jq . "$CLEANED_FILE" >/dev/null 2>&1; then
cp "$CLEANED_FILE" /tmp/invalid_json_debug.json
echo "❌ Error: JSON is still invalid after cleaning control characters"
echo "💡 Use 'cat /tmp/invalid_json_debug.json' to inspect the JSON"
exit 1
fi
PR_LIST_RESPONSE_FILE="$CLEANED_FILE"
fi


PR_MERGED_COUNT=$(echo "$PR_LIST_RESPONSE" | jq '.total_count')

# Color codes
Comment on lines +64 to +66
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: PR_MERGED_COUNT is derived from echo "$PR_LIST_RESPONSE" | jq '.total_count', which assumes jq is installed but the script never checks for it up front.

Why: On systems without jq, the script will fail mid-way with a non-obvious error, even though jq is a hard dependency for core functionality.

How to Fix: Add an early dependency check for jq (and optionally curl) near the top of the script, with a clear error message if it’s missing.

Suggested change
PR_MERGED_COUNT=$(echo "$PR_LIST_RESPONSE" | jq '.total_count')
# Color codes
if ! command -v jq >/dev/null 2>&1; then
echo "❌ Error: 'jq' is required but not installed. Please install jq to use this script."
exit 1
fi
PR_MERGED_COUNT=$(jq '.total_count' "$PR_LIST_RESPONSE_FILE")
# Color codes

GREEN='\033[0;32m'
YELLOW='\033[0;33m'
NOCOLOR='\033[0m'

echo "=============================================================================="
echo -e "Found ${GREEN}$PR_MERGED_COUNT${NOCOLOR} PR(s) merged into $SOURCE_BRANCH (filter: $DATE_FILTER)"
echo "=============================================================================="
echo -e "## ${GREEN}CHANGELOG${NOCOLOR}"
echo "$PR_LIST_RESPONSE" | jq -r '.items[] | (.title) + " (" + (.user.login) + ") [#" + (.number | tostring) + "]"' | sort
printf "\n"
Comment on lines +73 to +76
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: The script assumes .items always exists and is an array, and pipes directly to jq -r '.items[] | ...', which will fail with a non-zero exit code if there are no items or if the structure is slightly different.

Why: An empty result set (no merged PRs in the period) is a valid scenario and should not cause the script to error; instead, it should print an empty changelog or a friendly message.

How to Fix: Guard the iteration with a check on PR_MERGED_COUNT or use jq options that handle empty arrays gracefully, and print a clear message when there are zero PRs.

Suggested change
echo "=============================================================================="
echo -e "## ${GREEN}CHANGELOG${NOCOLOR}"
echo "$PR_LIST_RESPONSE" | jq -r '.items[] | (.title) + " (" + (.user.login) + ") [#" + (.number | tostring) + "]"' | sort
printf "\n"
echo "=============================================================================="
echo -e "## ${GREEN}CHANGELOG${NOCOLOR}"
if [ "$PR_MERGED_COUNT" -gt 0 ]; then
jq -r '.items[] | (.title) + " (" + (.user.login) + ") [#" + (.number | tostring) + "]"' "$PR_LIST_RESPONSE_FILE" | sort
else
echo "No merged PRs found for the specified branch and date filter."
fi
printf "\n"

echo "=============================================================================="
echo -e "${GREEN}GitHub Search URL (to verify, if required)${NOCOLOR}"
BRANCH_ENCODED=$(echo "$SOURCE_BRANCH" | sed 's/ /%20/g')
echo "https://github.com/$REPO/pulls?q=NOT+%22Parent+branch+sync%22+in%3Atitle+is%3Apr+is%3Amerged+base%3A$BRANCH_ENCODED+merged%3A$DATE_FILTER+-author%3Aapp%2Fdistributed-gitflow-app"
echo "=============================================================================="

# Clean up temporary files
rm -f /tmp/curl_output.json /tmp/curl_error.log
Comment on lines +83 to +84
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟢 LOW

Problem: The cleanup step removes only /tmp/curl_output.json and /tmp/curl_error.log, which becomes incorrect once temp files are made unique (e.g., via mktemp), and it doesn’t clean up any additional temp files created during JSON cleaning.

Why: Leaving temp files behind clutters /tmp and can leak response data; attempting to remove hardcoded paths that no longer exist is harmless but misleading and suggests incomplete cleanup.

How to Fix: Track all temp file paths in variables and remove those specific files at the end of the script (and optionally via a trap to ensure cleanup on error).

Suggested change
# Clean up temporary files
rm -f /tmp/curl_output.json /tmp/curl_error.log
# Clean up temporary files
rm -f "$OUTPUT_FILE" "$ERROR_FILE" ${CLEANED_FILE:-} /tmp/invalid_json_debug.json 2>/dev/null || true

13 changes: 13 additions & 0 deletions .github/workflows/pr-lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: Common PR Lint

on:
pull_request:
branches: [master, main,staging, dev,develop]
types: [ready_for_review, reopened, review_requested, review_request_removed, opened, edited]
Comment on lines +3 to +6
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: The branches list is missing spaces after some commas (main,staging, dev,develop), which is inconsistent and can reduce readability/maintainability of the workflow configuration.

Why: While GitHub Actions will still parse this correctly, inconsistent formatting makes the workflow harder to scan and increases the chance of subtle mistakes when editing (e.g., accidentally merging branch names).

How to Fix: Normalize the list formatting by adding spaces after all commas so each branch name is clearly separated and the style is consistent.

Suggested change
on:
pull_request:
branches: [master, main,staging, dev,develop]
types: [ready_for_review, reopened, review_requested, review_request_removed, opened, edited]
on:
pull_request:
branches: [master, main, staging, dev, develop]
types: [ready_for_review, reopened, review_requested, review_request_removed, opened, edited]


jobs:
pr-lint:
name: Common PR Lint Checks
if: github.base_ref == 'main' || github.base_ref == 'master'
uses: chargebee/cb-cicd-pipelines/.github/workflows/pr-lint.yml@main
secrets: inherit
60 changes: 60 additions & 0 deletions .github/workflows/pr-size-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
name: PR Size Check
on:
pull_request:
types: [ reopened, opened, synchronize, edited, labeled, unlabeled ]
branches:
- develop/**
- dev


jobs:
pre-approval-comment:
name: Announce pending bypass approval
if: ${{ github.event.pull_request.user.login != 'distributed-gitflow-app[bot]' &&
!startsWith(github.head_ref, 'revert-') &&
!startsWith(github.head_ref, 'parent-branch-sync/') &&
contains(github.event.pull_request.labels.*.name, 'pr-size-exception') }}
runs-on: graviton-small-runner
permissions:
contents: read
pull-requests: write
steps:
- uses: actions/github-script@v7
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const owner = context.repo.owner;
const repo = context.repo.repo;
const issue_number = context.payload.pull_request.number;

const marker = '<!-- pr-size-bypass-pending -->';
const pending = `${marker}
🛑 The \`pr-size-exception\` label is present. This workflow is **waiting for approvals** from the **[cb-Billing-CAB-reviewers](https://github.com/orgs/chargebee/teams/cb-billing-cab-approvers)**.`;

// create a new comment when the workflow runs
await github.rest.issues.createComment({ owner, repo, issue_number, body: pending });
pr-size-check:
name: Check PR size
if: ${{ (github.base_ref == 'dev' || startsWith(github.base_ref, 'develop/')) && github.event.pull_request.user.login != 'distributed-gitflow-app[bot]' && !startsWith(github.head_ref, 'revert-') && !startsWith(github.head_ref, 'parent-branch-sync/') }}
runs-on: graviton-small-runner
permissions:
contents: read
pull-requests: write
env:
BYPASS_LABEL: pr-size-exception
environment: ${{ contains(github.event.pull_request.labels.*.name, 'pr-size-exception') && 'cb-billing-reviewers' || '' }}
steps:
- uses: chargebee/cb-cicd-pipelines/.github/actions/pr-size-check@v4.20.3
Comment on lines +43 to +47
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟡 MEDIUM

Problem: The environment is set to an empty string ('') when the pr-size-exception label is not present, which is not a valid environment name and can cause the job to fail or behave unexpectedly.

Why: GitHub Actions expects environment to be either omitted or set to a valid environment name; providing an empty string may result in runtime errors or undefined behavior, breaking the PR size check workflow for normal PRs.

How to Fix: Make the environment assignment conditional so that it is only set when the bypass label is present, and omitted otherwise (e.g., using a ternary that returns null instead of '', or by moving the environment to a separate job or using if on the job).

Suggested change
env:
BYPASS_LABEL: pr-size-exception
environment: ${{ contains(github.event.pull_request.labels.*.name, 'pr-size-exception') && 'cb-billing-reviewers' || '' }}
steps:
- uses: chargebee/cb-cicd-pipelines/.github/actions/pr-size-check@v4.20.3
env:
BYPASS_LABEL: pr-size-exception
environment: ${{ contains(github.event.pull_request.labels.*.name, 'pr-size-exception') && 'cb-billing-reviewers' || null }}
steps:
- uses: chargebee/cb-cicd-pipelines/.github/actions/pr-size-check@v4.20.3

if: ${{ !contains(github.event.pull_request.labels.*.name, env.BYPASS_LABEL) }}
with:
githubToken: ${{ secrets.GITHUB_TOKEN }}
errorSize: 250
warningSize: 200
excludePaths: |
.github/**
.cursor/**


- name: Ensure required check passes when bypassed
Comment on lines +53 to +58
Copy link

@hivel-marco hivel-marco bot Mar 12, 2026

Choose a reason for hiding this comment

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

Priority: 🟢 LOW

Problem: There are two trailing blank lines inside the excludePaths block, which are unnecessary and can make the YAML less tidy or potentially confusing for future edits.

Why: While YAML will generally ignore these blank lines, keeping configuration files clean and minimal improves readability and reduces the chance of accidental indentation or formatting errors in future modifications.

How to Fix: Remove the extra blank lines at the end of the excludePaths list so that only the intended paths are present.

Suggested change
excludePaths: |
.github/**
.cursor/**
- name: Ensure required check passes when bypassed
excludePaths: |
.github/**
.cursor/**
- name: Ensure required check passes when bypassed

if: ${{ contains(github.event.pull_request.labels.*.name, env.BYPASS_LABEL) }}
run: echo "Bypass active — marking job successful."
Loading