Conversation
![hivel-marco[bot]](https://avatars.githubusercontent.com/in/2836183?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Complexity Score: 2.0 - Trivial
View Breakdown
- Lines Changed: 217
- Files Changed: 5
- Complexity Added: 0
- Raw Score: 19.34
⚠️ Sensitive Data Detected
| File | Types | Count | ||||||
|---|---|---|---|---|---|---|---|---|
|
| Line | Type | Preview |
|---|---|---|
| 13 | Secret: Secret Keyword | [Secret Keyword] |
High-level summary
This PR introduces standardized tooling and automation around pull requests and changelog generation:
- Adds a PR template to enforce consistent metadata (changelog, summary, test reports, impact areas, change type, documentation).
- Adds a script and documentation to generate changelogs from merged PRs via the GitHub API.
- Adds CI workflows for:
- Common PR linting (reusing a shared workflow).
- PR size checking with label-based bypass and CAB approval flow.
Key areas affected: .github configuration (PR templates, scripts, and workflows), CI behavior for PRs targeting main/master and dev/develop branches.
Key functionalities and changes
-
PR Template
- New standardized PR template capturing changelog, summary, automation status, test report URL, areas of impact, type of change, and documentation links.
- Encourages consistent PR metadata and classification (bugfix, feature, enhancement, tests, docs, chore).
-
Changelog Generation Script
- New
generate-changelog.shscript to list merged PRs into a given branch over a date range. - Filters out “Parent branch sync” and bot-authored PRs.
- Uses GitHub Search API with robust error handling (HTTP status checks, curl failures, JSON validation/cleanup).
- Outputs a formatted changelog and a GitHub search URL for verification.
- New
-
Changelog Script Documentation
- README describing prerequisites (
jq, GitHub credentials), environment setup, usage examples, and arguments.
- README describing prerequisites (
-
PR Lint Workflow
- New
Common PR Lintworkflow triggered on PR events for key branches. - Delegates checks to a shared workflow in
chargebee/cb-cicd-pipelines. - Only runs when base branch is
mainormaster.
- New
-
PR Size Check Workflow
- New
PR Size Checkworkflow for PRs targetingdevanddevelop/**. - Uses shared
pr-size-checkaction with thresholds (warning at 200, error at 250 lines). - Excludes
.github/**and.cursor/**from size calculation. - Supports a
pr-size-exceptionlabel:- Posts a comment indicating pending CAB approval.
- Sets an environment requiring
cb-billing-reviewerswhen bypass label is present. - Ensures the job passes when bypass is active so required checks succeed.
- New
File-level change summary
| File | Change Summary |
|---|---|
.github/pull_request_template.md |
Adds a standardized PR template including sections for changelog, summary, automation changes, test report URL, areas of impact, type of change (bugfix/feature/enhancement/tests/docs/chore), and documentation links. |
.github/scripts/README.md |
Introduces documentation for the changelog generation script: prerequisites, environment variable setup (GH_USERNAME, GH_PAT), usage examples for different branches/date filters, and explanation of arguments. |
.github/scripts/generate-changelog.sh |
Adds a Bash script to query GitHub’s Search API for merged PRs into a specified branch within a date range, excluding sync/bot PRs. Includes environment validation, cross-platform date handling, HTTP and curl error handling, JSON validation/cleanup with jq, formatted changelog output, and a verification URL. |
.github/workflows/pr-lint.yml |
Adds a reusable “Common PR Lint” GitHub Actions workflow that triggers on various PR events for master, main, staging, dev, and develop, but only runs when the base is main or master, delegating to chargebee/cb-cicd-pipelines and inheriting secrets. |
.github/workflows/pr-size-check.yml |
Adds a “PR Size Check” workflow for PRs into dev and develop/**. Implements a pre-approval comment when pr-size-exception is present, runs a shared pr-size-check action with size thresholds and path exclusions, and supports a bypass flow tied to the pr-size-exception label and cb-billing-reviewers environment. |
| ## DOCUMENTATION | ||
| REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA No newline at end of file |
There was a problem hiding this comment.
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 style violations in some environments.
How to Fix: Add a newline at the end of the file so the last line (REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA) is followed by a line break.
| ## DOCUMENTATION | |
| REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA | |
| ## DOCUMENTATION | |
| REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA |
| # 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-flutter" |
There was a problem hiding this comment.
Priority: 🟠 HIGH
Problem: The DATE_FILTER default uses date -v-30d (BSD/macOS) with a fallback to date -d '30 days ago' (GNU/Linux), but the entire command substitution is inside a single $(...), so if the first date exists but fails (e.g., different BSD semantics) or prints nothing, the fallback will never run and DATE_FILTER may be empty or invalid.
Why: An invalid or empty DATE_FILTER will produce an incorrect merged: query segment, which can cause the GitHub Search API call to fail or return unexpected results, breaking changelog generation in some environments.
How to Fix: Explicitly detect which date variant is supported and construct DATE_FILTER in separate conditional branches, ensuring a valid value is always produced or the script exits with a clear error.
| # 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-flutter" | |
| # 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) | |
| if [[ -z "$2" ]]; then | |
| if date -u -v-30d +%Y-%m-%d >/dev/null 2>&1; then | |
| LAST_30_DAYS=$(date -u -v-30d +%Y-%m-%d) | |
| elif date -u -d '30 days ago' +%Y-%m-%d >/dev/null 2>&1; then | |
| LAST_30_DAYS=$(date -u -d '30 days ago' +%Y-%m-%d) | |
| else | |
| echo "❌ Error: 'date' command does not support required options for computing last 30 days" | |
| exit 1 | |
| fi | |
| DATE_FILTER="merged:>=$LAST_30_DAYS" | |
| else | |
| DATE_FILTER="$2" | |
| fi | |
| # Repo is set per-repo when this file is pushed (placeholder replaced by upload script) | |
| REPO="chargebee/chargebee-flutter" |
| # Repo is set per-repo when this file is pushed (placeholder replaced by upload script) | ||
| REPO="chargebee/chargebee-flutter" | ||
|
|
||
| echo "🔍 Searching for PRs merged into $SOURCE_BRANCH..." |
There was a problem hiding this comment.
Priority: 🟡 MEDIUM
Problem: The REPO variable is hardcoded to chargebee/chargebee-flutter, which makes the script non-reusable across repositories and contradicts the comment that it is “set per-repo when this file is pushed.”
Why: If this script is copied to another repository without updating REPO, it will query and generate changelogs for the wrong repo, leading to misleading or incorrect release notes.
How to Fix: Make REPO configurable via an environment variable with a sensible default, or fail fast if it is not set, so each repository must explicitly define its own value.
| # Repo is set per-repo when this file is pushed (placeholder replaced by upload script) | |
| REPO="chargebee/chargebee-flutter" | |
| echo "🔍 Searching for PRs merged into $SOURCE_BRANCH..." | |
| # Repo is set per-repo when this file is pushed (placeholder replaced by upload script) | |
| REPO="${REPO:-chargebee/chargebee-flutter}" | |
| if [[ -z "$REPO" ]]; then | |
| echo "❌ Error: REPO is not set. Please export REPO as 'owner/repo'." | |
| exit 1 | |
| fi | |
| echo "🔍 Searching for PRs merged into $SOURCE_BRANCH..." |
| 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) |
There was a problem hiding this comment.
Priority: 🟡 MEDIUM
Problem: The GitHub Search API query string includes merged:$DATE_FILTER, but DATE_FILTER already contains the merged: prefix (e.g., merged:>=2024-01-01), resulting in merged:merged:>=2024-01-01 which is invalid or at least confusing for the search syntax.
Why: An incorrectly constructed search query can cause the API to return no results or unexpected results, undermining the reliability of the generated changelog.
How to Fix: Store only the date expression (e.g., >=2024-01-01) in DATE_FILTER and prepend merged: only where the query is constructed, or rename the variable to reflect that it already includes the merged: prefix and avoid adding it again.
| 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) | |
| 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) |
| 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" | ||
| echo "==============================================================================" |
There was a problem hiding this comment.
Priority: 🟡 MEDIUM
Problem: The script assumes that .items is always present and an array in the API response (jq -r '.items[] | ...'), but on error or unexpected responses this can fail with a non-zero exit code and terminate the script due to set -e.
Why: A malformed or rate-limited API response (e.g., missing items) will cause the script to crash instead of failing gracefully with a clear message, making it harder to diagnose issues.
How to Fix: Guard the jq extraction with a check that .items exists and is an array, or use jq -e with a conditional to print a helpful error when the expected structure is missing.
| 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" | |
| echo "==============================================================================" | |
| echo "==============================================================================" | |
| echo -e "Found ${GREEN}$PR_MERGED_COUNT${NOCOLOR} PR(s) merged into $SOURCE_BRANCH (filter: $DATE_FILTER)" | |
| echo "==============================================================================" | |
| echo -e "## ${GREEN}CHANGELOG${NOCOLOR}" | |
| if echo "$PR_LIST_RESPONSE" | jq -e '.items and (.items | type == "array")' >/dev/null 2>&1; then | |
| echo "$PR_LIST_RESPONSE" | jq -r '.items[] | (.title) + " (" + (.user.login) + ") [#" + (.number | tostring) + "]"' | sort | |
| else | |
| echo "⚠️ No valid 'items' array found in API response; raw response saved above." | |
| fi | |
| printf "\n" | |
| echo "==============================================================================" |
| 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 "==============================================================================" |
There was a problem hiding this comment.
Priority: 🟡 MEDIUM
Problem: The GitHub search URL uses $DATE_FILTER directly in merged%3A$DATE_FILTER, but DATE_FILTER may contain characters (e.g., >=) that are not URL-encoded, leading to an incorrect or broken link.
Why: An improperly encoded URL can confuse browsers or GitHub’s search parser, making the “verification” link unreliable for users.
How to Fix: URL-encode the merged: filter portion (or the entire query) before interpolating it into the URL, at least encoding characters like > and =.
| 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 "==============================================================================" | |
| echo "==============================================================================" | |
| echo -e "${GREEN}GitHub Search URL (to verify, if required)${NOCOLOR}" | |
| BRANCH_ENCODED=$(echo "$SOURCE_BRANCH" | sed 's/ /%20/g') | |
| DATE_FILTER_ENCODED=$(echo "$DATE_FILTER" | sed 's/>/%3E/g; s/=/%3D/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_ENCODED+-author%3Aapp%2Fdistributed-gitflow-app" | |
| echo "==============================================================================" |
| on: | ||
| pull_request: | ||
| branches: [master, main,staging, dev,develop] | ||
| types: [ready_for_review, reopened, review_requested, review_request_removed, opened, edited] |
There was a problem hiding this comment.
Priority: 🟡 MEDIUM
Problem: The branches array is missing spaces after some commas ([master, main,staging, dev,develop]), which is inconsistent and can reduce readability compared to the rest of the YAML in this repo.
Why: While GitHub Actions will still parse this correctly, inconsistent formatting makes the workflow harder to scan and maintain, especially as more branches or triggers are added.
How to Fix: Normalize the spacing in the branches list so all entries are separated by a comma and a space.
| 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] |
| 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 |
There was a problem hiding this comment.
Priority: 🟢 LOW
Problem: The if condition uses github.base_ref, which is only set for pull_request events, but it compares against 'main' and 'master' while the branches filter also includes staging, dev, and develop, making those extra branches effectively dead configuration.
Why: Having branches listed that can never satisfy the if condition introduces confusion for maintainers; it suggests the workflow should run for staging, dev, and develop, but it will never execute for those bases due to the explicit if filter.
How to Fix: Either remove the unused branches from the branches filter to reflect the actual behavior, or expand the if condition to include them if they are intended to be supported.
| 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 | |
| pr-lint: | |
| name: Common PR Lint Checks | |
| if: github.base_ref == 'main' || github.base_ref == 'master' || github.base_ref == 'staging' || github.base_ref == 'dev' || startsWith(github.base_ref, 'develop') | |
| uses: chargebee/cb-cicd-pipelines/.github/workflows/pr-lint.yml@main |
| env: | ||
| BYPASS_LABEL: pr-size-exception | ||
| environment: ${{ contains(github.event.pull_request.labels.*.name, 'pr-size-exception') && 'cb-billing-reviewers' || '' }} | ||
| steps: |
There was a problem hiding this comment.
Priority: 🟠 HIGH
Problem: The environment value uses a conditional expression that can resolve to an empty string (''), which is not a valid environment name and may cause the workflow/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 is not documented behavior and can lead to runtime errors or confusing behavior when the bypass label is not present.
How to Fix: Only set the environment key when the pr-size-exception label is present, and omit it entirely otherwise by using an if condition on the job instead of a ternary expression in the environment field.
| env: | |
| BYPASS_LABEL: pr-size-exception | |
| environment: ${{ contains(github.event.pull_request.labels.*.name, 'pr-size-exception') && 'cb-billing-reviewers' || '' }} | |
| steps: | |
| env: | |
| BYPASS_LABEL: pr-size-exception | |
| 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/') }} | |
| environment: cb-billing-reviewers | |
| steps: |
| 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 }); |
There was a problem hiding this comment.
Priority: 🟡 MEDIUM
Problem: The pre-approval-comment job posts a new comment on every run when the pr-size-exception label is present, which will spam the PR with duplicate “pending approval” comments.
Why: Repeated comments on every synchronize/edited/labeled event will clutter the PR discussion and make it harder for reviewers to find relevant information.
How to Fix: Before creating a new comment, query existing comments and only post if a comment containing the marker (<!-- pr-size-bypass-pending -->) does not already exist.
| 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 }); | |
| 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)**.`; | |
| // Check for an existing pending comment to avoid duplicates | |
| const { data: comments } = await github.rest.issues.listComments({ | |
| owner, | |
| repo, | |
| issue_number, | |
| per_page: 100, | |
| }); | |
| const alreadyPending = comments.some(comment => comment.body && comment.body.includes(marker)); | |
| if (!alreadyPending) { | |
| await github.rest.issues.createComment({ owner, repo, issue_number, body: 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)**.`; |
There was a problem hiding this comment.
Priority: 🟢 LOW
Problem: The comment text references the team as cb-Billing-CAB-reviewers in the visible label but links to cb-billing-cab-approvers, which is inconsistent and may confuse contributors about the correct team.
Why: Inconsistent naming between the displayed team name and the linked team can cause confusion when trying to identify or contact the correct approvers.
How to Fix: Align the visible team name with the linked team slug, or adjust both to the correct canonical team name used in your organization.
| 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)**.`; | |
| const pending = `${marker} | |
| 🛑 The \`pr-size-exception\` label is present. This workflow is **waiting for approvals** from the **[cb-billing-cab-approvers](https://github.com/orgs/chargebee/teams/cb-billing-cab-approvers)**.`; |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
1 participant
This PR adds standardized PR template, changelog script, PR lint workflow, and PR size check workflow.