Quality Gates: PR Title standardization and LOC#95
Quality Gates: PR Title standardization and LOC#95cb-anomitromunshi wants to merge 6 commits intomasterfrom
Conversation
✅ 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. |
There was a problem hiding this comment.
PR Complexity Score: 2.0 - Simple
View Breakdown
- Lines Changed: 239
- Files Changed: 5
- Complexity Added: 0
- Raw Score: 19.78
⚠️ Sensitive Data Detected
| File | Types | Count | ||||||
|---|---|---|---|---|---|---|---|---|
|
| Line | Type | Preview |
|---|---|---|
| 13 | Secret: Secret Keyword | [Secret Keyword] |
.github/workflows/pr-size-check.yml
| Line | Type | Preview |
|---|---|---|
| 67 | PII: Phone Number | 127322177288 |
- Adds a standardized pull request template to enforce consistent metadata (changelog, summary, test reports, areas of impact, type of change, documentation).
- Introduces a script and documentation to generate changelogs from merged PRs via the GitHub API.
- Adds a reusable PR lint workflow that delegates checks to a shared CI/CD pipeline for PRs targeting
main/master. - Adds a PR size check workflow for
dev/develop/**branches, including:- Size thresholds and path exclusions.
- A
pr-size-exceptionbypass mechanism. - Commenting to indicate pending CAB approvals.
- Recording bypass approvals to S3 via AWS OIDC.
High-level summary
This PR introduces common GitHub workflow and tooling standards for the repository:
- Purpose: Improve PR quality, consistency, and governance by enforcing templates, linting, and size controls, and by enabling automated changelog generation.
- Key areas affected: GitHub configuration (
.github), CI workflows, and developer tooling for release notes.
File-level change summary
| File | Change summary |
|---|---|
.github/pull_request_template.md |
Adds a structured PR template capturing changelog, summary, automation status, test report URL, areas of impact, change type (bugfix/feature/enhancement/tests/docs/chore), and documentation links. |
.github/scripts/README.md |
Documents how to use the new generate-changelog.sh script, including required env vars (GH_USERNAME, GH_PAT), usage examples, and argument behavior. |
.github/scripts/generate-changelog.sh |
New Bash script that queries the GitHub API for merged PRs into a given branch over a date range, filters out sync/bot PRs, handles JSON/HTTP errors robustly, and prints a formatted changelog plus a verification search URL. |
.github/workflows/pr-lint.yml |
Adds a “Common PR Lint” workflow that runs on various PR events for key branches and delegates lint checks to chargebee/cb-cicd-pipelines when the base branch is main or master. |
.github/workflows/pr-size-check.yml |
Adds a “PR Size Check” workflow for PRs into dev/develop/**, enforcing size thresholds, excluding certain paths, supporting a pr-size-exception label with CAB approval flow, posting status comments, and recording bypass approvals to S3 using AWS OIDC. |
| ## 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 linters and tooling (including some Git-related checks and POSIX text file conventions) expect a newline at the end of files; missing it can cause noisy diffs and minor interoperability issues.
How to Fix: Add a newline at the end of the file after the last line so the file ends with a proper line terminator.
| ## DOCUMENTATION | |
| REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA | |
| ## DOCUMENTATION | |
| REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA |
|
|
||
| **Prerequisites:** `jq`, and GitHub credentials as env vars. | ||
|
|
||
| ### Set credentials (once per terminal) |
There was a problem hiding this comment.
Priority: 🟡 MEDIUM
Problem: The documentation instructs users to set GH_PAT to a GitHub personal access token without any guidance on scope or security, which can encourage over-scoped or insecure token usage.
Why: Using a PAT with excessive scopes or storing it in shell history/environment without guidance increases the risk of credential leakage and repository/account compromise, especially in shared or CI environments.
How to Fix: Clarify that the PAT should be minimally scoped (e.g., repo read-only as needed), recommend using GitHub CLI (gh auth) or environment managers where possible, and add a brief security note about not committing or sharing the token.
| ### Set credentials (once per terminal) | |
| ### Set credentials (once per terminal) |
| # 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-ios" |
There was a problem hiding this comment.
Priority: 🟠 HIGH
Problem: The DATE_FILTER construction mixes Git date syntax (merged:>=YYYY-MM-DD) with GitHub Search API syntax (merged:YYYY-MM-DD..YYYY-MM-DD), so the query merged:$DATE_FILTER is likely invalid and may return incorrect or no results.
Why: The GitHub Search API for issues/PRs does not support merged:>=YYYY-MM-DD as a range operator; instead it expects merged:YYYY-MM-DD..* or merged:>YYYY-MM-DD. Using an unsupported format can silently break changelog generation or produce incomplete data, undermining the purpose of the script.
How to Fix: Normalize DATE_FILTER to a plain date (e.g., YYYY-MM-DD) and build a valid GitHub search range in the query, such as merged:DATE..* or merged:>DATE. This keeps the date computation logic while ensuring the search query is syntactically correct for the GitHub API.
| # 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-ios" | |
| # 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) | |
| # Store just the date; build the GitHub search range in the query | |
| DATE_FILTER="${2:-$(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-ios" |
| 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 search query uses merged:$DATE_FILTER directly, but DATE_FILTER is a raw date (or previously a malformed merged:>=...), so the query does not explicitly define a proper range (e.g., merged:DATE..*) and may not match the intended "last 30 days" window.
Why: Without a correct range operator, the GitHub Search API may interpret the merged: qualifier incorrectly or not at all, leading to missing or extra PRs in the generated changelog. This reduces trust in the automation and can cause release notes to be incomplete.
How to Fix: Interpolate DATE_FILTER into a valid GitHub search range expression such as merged:$DATE_FILTER..* (from that date to present) or merged:>$DATE_FILTER. This makes the time window explicit and aligned with GitHub's documented search syntax.
| 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 "${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 verification GitHub Search URL uses merged%3A$DATE_FILTER directly, which does not match the fixed query semantics (e.g., merged:DATE..*), so the "verify" URL may show different results than the API call.
Why: If the human-facing verification URL does not mirror the exact search used by the script, developers may see discrepancies between the changelog output and the browser results, making debugging and trust in the tool harder.
How to Fix: Update the verification URL to use the same merged:DATE..* pattern (or whatever final range syntax is used in the API call) so that the browser search reproduces the script's behavior.
| 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') | |
| 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 "==============================================================================" |
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| # Check required environment variables | ||
| if [[ -z "$GH_USERNAME" ]]; then |
There was a problem hiding this comment.
Priority: 🟢 LOW
Problem: The script uses set -e without -u, so unset variables (e.g., if git branch --show-current fails or other env vars are referenced later) will not cause an immediate exit and may lead to confusing behavior.
Why: In shell scripts that depend on environment variables and command substitution, failing fast on unset variables (set -u) improves robustness and makes debugging easier, especially in CI environments.
How to Fix: Add -u (and optionally -o pipefail) to the shell options to ensure the script exits on unset variables and pipeline failures.
| #!/bin/bash | |
| set -e | |
| # Check required environment variables | |
| if [[ -z "$GH_USERNAME" ]]; then | |
| #!/bin/bash | |
| set -euo pipefail | |
| # Check required environment variables | |
| if [[ -z "$GH_USERNAME" ]]; then |
| 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 list in the pull_request trigger 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 YAML correctly, inconsistent formatting makes the workflow harder to scan and increases the chance of subtle mistakes when future contributors edit or extend the branch list.
How to Fix: Add spaces after all commas in the branches array to keep the list consistently formatted and easier to maintain.
| 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: 🟡 MEDIUM
Problem: The if condition uses github.base_ref, which is only set for pull_request events targeting a branch in the same repository; for some PR event variants or future trigger changes, this could be brittle compared to using github.event.pull_request.base.ref.
Why: Relying on the shorthand github.base_ref can make the workflow less explicit and slightly more fragile if triggers are expanded or behavior changes; using the full event path is clearer and aligns with common GitHub Actions best practices.
How to Fix: Update the condition to reference github.event.pull_request.base.ref so the base branch is read directly from the event payload.
| 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.event.pull_request.base.ref == 'main' || github.event.pull_request.base.ref == 'master' | |
| uses: chargebee/cb-cicd-pipelines/.github/workflows/pr-lint.yml@main |
| - 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: 🟠 HIGH
Problem: The pre-approval-comment job posts a new "pending bypass approval" comment on every run without checking for an existing marker, causing duplicate comments on the PR.
Why: On every PR event that matches the if condition (e.g., synchronize, labeled, edited), this will keep adding identical comments, cluttering the PR discussion and making it harder to see real review feedback.
How to Fix: Before creating a new comment, list existing comments on the PR and only create a new one if no comment contains the marker string.
| - 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 }); | |
| - 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)**.`; | |
| // Avoid duplicate comments by checking for an existing marker | |
| const { data: comments } = await github.rest.issues.listComments({ | |
| owner, | |
| repo, | |
| issue_number, | |
| per_page: 100, | |
| }); | |
| const alreadyPresent = comments.some(c => c.body && c.body.includes(marker)); | |
| if (!alreadyPresent) { | |
| await github.rest.issues.createComment({ owner, repo, issue_number, body: pending }); | |
| } |
| 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: 🟡 MEDIUM
Problem: The environment is set to an empty string ('') when the bypass label is not present, which can cause GitHub Actions to treat it as a named environment with an empty name or behave unexpectedly.
Why: GitHub environments are optional; passing an empty string is unnecessary and may lead to confusing environment listings or misconfiguration. It’s clearer and safer to only set environment when actually needed (i.e., when the bypass label is present).
How to Fix: Use a conditional expression that omits the environment key when the label is absent, or split the job into two variants. In this context, the simplest is to use a conditional expression that evaluates to null instead of ''.
| 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 | |
| environment: ${{ contains(github.event.pull_request.labels.*.name, 'pr-size-exception') && 'cb-billing-reviewers' || null }} | |
| steps: |
| warningSize: 200 | ||
| excludePaths: | | ||
| .github/** | ||
| .cursor/** | ||
|
|
||
|
|
||
| - name: Ensure required check passes when bypassed |
There was a problem hiding this comment.
Priority: 🟡 MEDIUM
Problem: The excludePaths list for the size check omits common non-code directories like docs/** or *.md, which may cause documentation-only or metadata-heavy PRs to be flagged as large, contrary to the intent of focusing on code changes.
Why: Including documentation and similar non-functional changes in size calculations can generate noisy warnings/errors and encourage unnecessary bypass labels, reducing the signal of the size check.
How to Fix: Extend excludePaths to cover typical non-code paths (e.g., docs/**, **/*.md, **/*.mdx) if those should not count toward PR size in this repository.
| warningSize: 200 | |
| excludePaths: | | |
| .github/** | |
| .cursor/** | |
| - name: Ensure required check passes when bypassed | |
| warningSize: 200 | |
| excludePaths: | | |
| .github/** | |
| .cursor/** | |
| docs/** | |
| **/*.md | |
| **/*.mdx | |
| - name: Ensure required check passes when bypassed |
There was a problem hiding this comment.
PR Complexity Score: 1.2 - Trivial
View Breakdown
- Lines Changed: 81
- Files Changed: 1
- Complexity Added: 0
- Raw Score: 4.62
⚠️ Sensitive Data Detected
| File | Types | Count | ||||||
|---|---|---|---|---|---|---|---|---|
|
| Line | Type | Preview |
|---|---|---|
| 13 | Secret: Secret Keyword | [Secret Keyword] |
.github/workflows/pr-size-check.yml
| Line | Type | Preview |
|---|---|---|
| 66 | PII: Phone Number | 127322177288 |
High-level summary
This PR introduces an automated GitHub Actions workflow to enforce and track pull request size limits on the master branch. It:
- Automatically checks PR size and flags large changes.
- Supports a
pr-size-exceptionbypass label with special handling and approvals. - Records bypass approvals to S3 for auditability.
- Posts a comment when a bypass is requested and pending approval.
Key functionalities and changes
- Added a new
PR Size CheckGitHub Actions workflow:- Triggers on PR events (
opened,reopened,synchronize,edited,labeled,unlabeled) targetingmaster. - Skips automated/system PRs and specific branch patterns (
revert-*,parent-branch-sync/*).
- Triggers on PR events (
- Implemented a pre-approval comment job:
- Runs when
pr-size-exceptionlabel is present. - Posts a standardized comment indicating that CAB approval is required from
cb-Billing-CAB-reviewers.
- Runs when
- Implemented the PR size check job:
- Uses
chargebee/cb-cicd-pipelines/.github/actions/pr-size-check@v4.20.3. - Enforces thresholds: warning at 200 lines, error at 250 lines.
- Excludes
.github/**and.cursor/**paths from size calculation. - When
pr-size-exceptionis present:- Marks the job successful to satisfy required checks.
- Configures AWS OIDC credentials.
- Records bypass approval metadata (repo, date, PR link, workflow ID) to an S3 bucket (
prsizebypassdata) for auditing.
- Uses
File-level change summary
| File | Change Summary |
|---|---|
.github/workflows/pr-size-check.yml |
New workflow that enforces PR size limits on master, posts pending-approval comments when pr-size-exception is used, conditionally runs a size check action, and when bypassed, configures AWS OIDC and records bypass approval details to S3 for audit and tracking. |
| DATE="$(date -u +%Y-%m-%dT%H:%M:%SZ)" | ||
| WF_ID="${{ github.run_id }}" | ||
| S3_KEY="${REPO}/datas/${WF_ID}.json" | ||
| printf '{"repo":"%s","date":"%s","pr_link":"%s","wf_id":"%s"}\n' "$REPO" "$DATE" "$PR_LINK" "$WF_ID" | \ | ||
| aws s3 cp - "s3://prsizebypassdata/${S3_KEY}" --content-type application/json | ||
| echo "Recorded to s3://prsizebypassdata/${S3_KEY}" |
There was a problem hiding this comment.
Priority: 🟡 MEDIUM
Problem: The S3 key path uses datas ("${REPO}/datas/${WF_ID}.json") which is likely a typo and may not match the intended or existing S3 folder structure for audit data.
Why: A misspelled or inconsistent prefix can scatter audit records across unexpected paths, making it harder to query or aggregate bypass approvals and potentially breaking any downstream reporting that expects a specific prefix.
How to Fix: Rename the folder segment to a consistent and meaningful name (e.g., data or bypasses) that matches your S3 layout conventions.
| DATE="$(date -u +%Y-%m-%dT%H:%M:%SZ)" | |
| WF_ID="${{ github.run_id }}" | |
| S3_KEY="${REPO}/datas/${WF_ID}.json" | |
| printf '{"repo":"%s","date":"%s","pr_link":"%s","wf_id":"%s"}\n' "$REPO" "$DATE" "$PR_LINK" "$WF_ID" | \ | |
| aws s3 cp - "s3://prsizebypassdata/${S3_KEY}" --content-type application/json | |
| echo "Recorded to s3://prsizebypassdata/${S3_KEY}" | |
| DATE="$(date -u +%Y-%m-%dT%H:%M:%SZ)" | |
| WF_ID="${{ github.run_id }}" | |
| S3_KEY="${REPO}/data/${WF_ID}.json" | |
| printf '{"repo":"%s","date":"%s","pr_link":"%s","wf_id":"%s"}\n' "$REPO" "$DATE" "$PR_LINK" "$WF_ID" | \ | |
| aws s3 cp - "s3://prsizebypassdata/${S3_KEY}" --content-type application/json | |
| echo "Recorded to s3://prsizebypassdata/${S3_KEY}" |
This PR adds standardized PR template, changelog script, PR lint workflow, and PR size check workflow.