-
Notifications
You must be signed in to change notification settings - Fork 12
PR standardization #94
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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: | ||||||||||||||||||||||||||||||
|
Comment on lines
+7
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟢 LOW Problem: The “FUNCTIONAL AUTOMATION CHANGES PR” section uses nested free-text prompts (“If Yes, PR:” / “If No, Reason:”) without a clear placeholder format, which can lead to inconsistent or incomplete entries. Why: Inconsistent formatting makes it harder to parse this information manually or via tooling, and contributors may leave these lines blank or unclear, reducing the value of the metadata. How to Fix: Add explicit placeholders (e.g.,
Suggested change
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ## AUTOMATION TEST REPORT URL | ||||||||||||||||||||||||||||||
| REPLACE_ME_WITH_TEST_REPORT_URL | ||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟢 LOW Problem: The “AUTOMATION TEST REPORT URL” section requires a URL but does not indicate what to do when no report exists (e.g., for small changes or docs-only PRs). Why: Lack of guidance can lead to inconsistent entries (left blank, “N/A”, or random text), which complicates automated checks or manual review expectations. How to Fix: Update the placeholder to explicitly allow
Suggested change
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ## AREAS OF IMPACT | ||||||||||||||||||||||||||||||
| REPLACE_ME_WITH_AREAS_OF_IMPACT_OR_NA | ||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟢 LOW Problem: The “AREAS OF IMPACT” placeholder suggests Why: Unstructured impact descriptions reduce the usefulness of this field for reviewers trying to quickly understand blast radius and for any future automation that might parse this section. How to Fix: Clarify the expected format (e.g., comma-separated components or example categories) in the placeholder text.
Suggested change
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ## TYPE OF CHANGE | ||||||||||||||||||||||||||||||
| - [ ] 🐞 Bugfix | ||||||||||||||||||||||||||||||
| - [ ] 🌟 Feature | ||||||||||||||||||||||||||||||
| - [ ] ✨ Enhancement | ||||||||||||||||||||||||||||||
| - [ ] 🧪 Unit Test Cases | ||||||||||||||||||||||||||||||
| - [ ] 📔 Documentation | ||||||||||||||||||||||||||||||
| - [ ] ⚙️ Chore - Build Related / Configuration / Others | ||||||||||||||||||||||||||||||
|
Comment on lines
+19
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟢 LOW Problem: The “TYPE OF CHANGE” checklist uses emojis, which may not render consistently in all environments and can make automated parsing of change types more difficult. Why: If future tooling needs to parse this section (e.g., for changelog generation or metrics), emojis mixed with labels can complicate reliable extraction of the selected type. How to Fix: Keep the human-readable labels but move emojis to the end or remove them, ensuring the leading text is a clean, parseable category name.
Suggested change
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ## DOCUMENTATION | ||||||||||||||||||||||||||||||
| REPLACE_ME_WITH_DOCUMENTATION_LINK_OR_NA | ||||||||||||||||||||||||||||||
|
Comment on lines
+28
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟢 LOW Problem: The file currently has “No newline at end of file”, which is a minor formatting issue and can cause noisy diffs or warnings in some tools. Why: POSIX and many linters expect text files to end with a newline; missing it can lead to inconsistent behavior across editors and minor friction in future diffs. How to Fix: Add a trailing newline at the end of the file.
Suggested change
|
||||||||||||||||||||||||||||||
| 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. |
| 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-ios" | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟡 MEDIUM Problem: The Why: This script is likely to be run in different shells/OSes (e.g., macOS with BSD How to Fix: Compute the default date in a separate variable with explicit error handling, then build
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 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) | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+27
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟡 MEDIUM Problem: The GitHub search query uses Why: A malformed How to Fix: Pass
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| if [ "$HTTP_STATUS" -ne 200 ]; then | ||||||||||||||||||||||||||||||||||||||||
| echo "❌ Error: API returned HTTP status $HTTP_STATUS" | ||||||||||||||||||||||||||||||||||||||||
| echo "Response: $(cat /tmp/curl_output.json)" | ||||||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| PR_LIST_RESPONSE=$(cat /tmp/curl_output.json) | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| # 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 | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| PR_MERGED_COUNT=$(echo "$PR_LIST_RESPONSE" | jq '.total_count') | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| # 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
+71
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟡 MEDIUM Problem: The script pipes Why: When no PRs match the search criteria, How to Fix: Check
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
| 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 "==============================================================================" | ||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+77
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟢 LOW Problem: The manual URL construction only encodes spaces in Why: While GitHub’s web UI is forgiving, unencoded special characters in query parameters can lead to incorrect parsing or require manual fixing by the user, reducing the reliability of the “verification URL” feature. How to Fix: URL-encode both the branch and the full query string (or at least the dynamic parts) before interpolating them into the URL, using
Suggested change
|
||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| # Clean up temporary files | ||||||||||||||||||||||||||||||||||||||||
| rm -f /tmp/curl_output.json /tmp/curl_error.log | ||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟡 MEDIUM Problem: The Why: While GitHub Actions accepts this syntax, inconsistent formatting makes the workflow harder to scan and increases the chance of subtle mistakes when branches are added/edited later. How to Fix: Add spaces after all commas in the
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 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 | ||||||||||||||||||||||
|
Comment on lines
+8
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟢 LOW Problem: The Why: This mismatch between the trigger branches and the conditional execution can be confusing for maintainers and may lead to incorrect assumptions that lint checks run on all configured branches when they actually do not. How to Fix: Either narrow the
Suggested change
|
||||||||||||||||||||||
| secrets: inherit | ||||||||||||||||||||||
| 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 }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟡 MEDIUM Problem: The Why: Repeated identical comments can clutter the PR discussion, make it harder for reviewers to find relevant information, and may annoy contributors; this is especially likely because the workflow triggers on multiple PR events (synchronize, edited, labeled, unlabeled). How to Fix: Before creating a new comment, query existing comments on the PR and only post if a comment containing the marker (
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟠 HIGH Problem: The Why: GitHub Actions expects How to Fix: Only set the
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Priority: 🟢 LOW Problem: There are two trailing blank lines within the Why: While this does not affect functionality, extra blank lines inside YAML blocks can make the configuration look untidy and may confuse future editors about whether additional values are intended. How to Fix: Remove the redundant blank lines so that the
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: ${{ contains(github.event.pull_request.labels.*.name, env.BYPASS_LABEL) }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: echo "Bypass active — marking job successful." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Priority: 🟢 LOW
Problem: The top-level section is titled
## CHANGELOG, but the template body and PR description emphasize capturing both a changelog and a summary; using only "CHANGELOG" here may confuse authors about where to put a concise summary vs. detailed changelog entry.Why: Ambiguous section naming can lead to inconsistent usage of the template, with some authors putting only a summary here and skipping the dedicated
SUMMARYsection, reducing the effectiveness of standardized metadata.How to Fix: Rename the section to clearly indicate it is for the changelog entry (e.g., “Changelog Entry”) or add clarifying text so authors understand this is the line that will be copied into release notes.