Conversation
📝 WalkthroughWalkthroughA new package size reporting system is introduced for Android artifacts. It includes a Bash script that builds release artifacts and generates per-module size reports in either Markdown or text format, along with GitHub Actions workflows to integrate this reporting into CI/CD and make it reusable across workflow runs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/package-sizes-report.yml:
- Around line 125-163: The index build currently uses only the local
site/reports directory and overwrites site/index.html, dropping previously
published reports from gh-pages; update the workflow so it first fetches or
checks out the existing gh-pages content into site (or merges the remote
gh-pages reports into site/reports) before generating site/index.html, then
generate the index by enumerating the combined site/reports (the same logic
around the find site/reports loop) so historical report links are preserved when
the site is published.
- Line 57: The workflow invokes the wrong script: replace the call to
./measure-android-package-sizes.sh with the actual script name
package-sizes-report.sh (i.e., invoke ./package-sizes-report.sh >
"${REPORT_DIR}/package-size-report.md" or rename the script to match), updating
the workflow step that writes to "${REPORT_DIR}/package-size-report.md" so the
job runs the correct script.
- Around line 59-67: The Python step fails because KIND and REPORT_DIR are read
from os.environ but those shell variables are not exported; ensure KIND and
REPORT_DIR are available to the python subprocess by exporting them or moving
them into the step's env mapping: either add export KIND and export REPORT_DIR
before invoking python, or add KIND and REPORT_DIR to the action step's env
(alongside REF) so the Python code (which reads REF, KIND, REPORT_DIR and writes
md_path) can access them.
In `@package-sizes-report.sh`:
- Around line 36-39: The build_release_artifacts function currently calls
./gradlew which fails when the script is run from outside the repo root; update
build_release_artifacts to invoke Gradle via the computed ROOT_DIR (use the
ROOT_DIR variable to build the gradlew path, e.g. ROOT_DIR/gradlew) so the
script runs reliably regardless of CWD, preserving the existing echo and output
redirection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 866cf1a9-3603-48fb-8870-eba1c461fbff
📒 Files selected for processing (2)
.github/workflows/package-sizes-report.ymlpackage-sizes-report.sh
|
|
||
| mkdir -p "${REPORT_DIR}" | ||
|
|
||
| ./measure-android-package-sizes.sh markdown > "${REPORT_DIR}/package-size-report.md" |
There was a problem hiding this comment.
Workflow invokes the wrong script filename.
Line 57 calls ./measure-android-package-sizes.sh, but the added script is package-sizes-report.sh. This will fail the job before report generation.
Suggested fix
- ./measure-android-package-sizes.sh markdown > "${REPORT_DIR}/package-size-report.md"
+ ./package-sizes-report.sh markdown > "${REPORT_DIR}/package-size-report.md"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/package-sizes-report.yml at line 57, The workflow invokes
the wrong script: replace the call to ./measure-android-package-sizes.sh with
the actual script name package-sizes-report.sh (i.e., invoke
./package-sizes-report.sh > "${REPORT_DIR}/package-size-report.md" or rename the
script to match), updating the workflow step that writes to
"${REPORT_DIR}/package-size-report.md" so the job runs the correct script.
| python3 - <<'PY' | ||
| import html | ||
| import os | ||
| from pathlib import Path | ||
|
|
||
| ref = os.environ["REF"] | ||
| kind = os.environ["KIND"] | ||
| report_dir = Path(os.environ["REPORT_DIR"]) | ||
| md_path = report_dir / "package-size-report.md" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "package-sizes-report.yml" -type fRepository: ForgeRock/ping-android-sdk
Length of output: 114
🏁 Script executed:
cat -n .github/workflows/package-sizes-report.yml | sed -n '50,130p'Repository: ForgeRock/ping-android-sdk
Length of output: 3440
🏁 Script executed:
cat -n .github/workflows/package-sizes-report.yml | head -50Repository: ForgeRock/ping-android-sdk
Length of output: 1534
Python step will fail: KIND and REPORT_DIR are not exported.
Lines 64-67 read KIND and REPORT_DIR from os.environ, but they are only defined as shell variables (lines 46-48 and 53) without being exported. The env: section (lines 122-124) only provides REF. When the Python subprocess runs, it cannot access non-exported shell variables, causing KeyError.
Suggested fix
REF="${{ inputs.ref }}"
# ... KIND determination logic ...
REPORT_DIR="site/reports/${KIND}-${SLUG}"
+ export REF KIND REPORT_DIR🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/package-sizes-report.yml around lines 59 - 67, The Python
step fails because KIND and REPORT_DIR are read from os.environ but those shell
variables are not exported; ensure KIND and REPORT_DIR are available to the
python subprocess by exporting them or moving them into the step's env mapping:
either add export KIND and export REPORT_DIR before invoking python, or add KIND
and REPORT_DIR to the action step's env (alongside REF) so the Python code
(which reads REF, KIND, REPORT_DIR and writes md_path) can access them.
| - name: Build root index | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| mkdir -p site | ||
|
|
||
| cat > site/index.html <<'EOF' | ||
| <!doctype html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1"> | ||
| <title>Android package size reports</title> | ||
| <style> | ||
| body { | ||
| font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Helvetica, Arial, sans-serif; | ||
| margin: 40px auto; | ||
| max-width: 1100px; | ||
| padding: 0 20px; | ||
| line-height: 1.5; | ||
| color: #24292f; | ||
| } | ||
| a { color: #0969da; text-decoration: none; } | ||
| a:hover { text-decoration: underline; } | ||
| li { margin: 8px 0; } | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <h1>Android package size reports</h1> | ||
| <p>This site contains package size reports published from manually triggered GitHub Actions runs.</p> | ||
| <ul> | ||
| EOF | ||
|
|
||
| if [ -d site/reports ]; then | ||
| find site/reports -mindepth 1 -maxdepth 1 -type d | sort | while read -r d; do | ||
| name="$(basename "$d")" | ||
| printf ' <li><a href="reports/%s/">%s</a></li>\n' "$name" "$name" >> site/index.html | ||
| done | ||
| fi |
There was a problem hiding this comment.
Root index generation drops historical report links.
The index is built from local site/reports before merging with existing gh-pages content. That typically includes only the current run, so previously published report links disappear from site/index.html.
Suggested fix direction
- - name: Build root index
- shell: bash
- run: |
- ...
- if [ -d site/reports ]; then
- find site/reports ...
- fi
+ # Build index after copying into gh-pages, and enumerate gh-pages/reports
+ # so all historical report directories remain listed.Also applies to: 193-200
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/package-sizes-report.yml around lines 125 - 163, The index
build currently uses only the local site/reports directory and overwrites
site/index.html, dropping previously published reports from gh-pages; update the
workflow so it first fetches or checks out the existing gh-pages content into
site (or merges the remote gh-pages reports into site/reports) before generating
site/index.html, then generate the index by enumerating the combined
site/reports (the same logic around the find site/reports loop) so historical
report links are preserved when the site is published.
spetrov
force-pushed
the
SDKS-4216
branch
2 times, most recently
from
0543bc6 to
ec279e8
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #185 +/- ##
=============================================
+ Coverage 43.98% 44.00% +0.02%
Complexity 1231 1231
=============================================
Files 306 306
Lines 9255 9255
Branches 1326 1326
=============================================
+ Hits 4071 4073 +2
Misses 4638 4638
+ Partials 546 544 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
spetrov
force-pushed
the
SDKS-4216
branch
2 times, most recently
from
6602a42 to
05e5b7b
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.github/workflows/package-sizes.yml (1)
14-18: Unnecessary full history fetch.
fetch-depth: 0pulls the entire git history, which isn't needed to build release artifacts and measure file sizes. Since the checkout is only used to run Gradle + the size script, a shallow clone (the defaultfetch-depth: 1) is sufficient and noticeably faster on a monorepo. Similarly, the explicitref: ${{ github.ref }}duplicates the default behavior and can be dropped.♻️ Proposed simplification
- name: Checkout current ref uses: actions/checkout@v4 - with: - ref: ${{ github.ref }} - fetch-depth: 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/package-sizes.yml around lines 14 - 18, Remove the unnecessary full-history checkout options: delete the explicit ref: ${{ github.ref }} line and remove or change fetch-depth: 0 to the default shallow clone (omit fetch-depth or set fetch-depth: 1) in the actions/checkout@v4 step so the workflow uses a faster shallow clone for the package-sizes job.package-sizes-report.sh (2)
18-21: Passbytesinto awk as a variable rather than interpolating into the script.Building the awk program via shell string interpolation is a fragile pattern: any non-numeric value in
${bytes}would turn into awk syntax instead of data, and it also trips static analyzers. Using-vis safer and idiomatic.♻️ Proposed fix
to_kb() { local bytes="$1" - awk "BEGIN { printf \"%.1f\", ${bytes}/1024 }" + awk -v b="${bytes}" 'BEGIN { printf "%.1f", b/1024 }' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package-sizes-report.sh` around lines 18 - 21, The to_kb function currently interpolates ${bytes} directly into the awk program which is unsafe; change it to pass bytes into awk via the -v option (e.g., -v b="$bytes") and use b inside the awk script (printf "%.1f", b/1024) so the numeric value is treated as data, not code, and avoids shell interpolation issues when calling to_kb.
23-28: Quote the expansion inside the parameter pattern (ShellCheck SC2295).In
${path#${ROOT_DIR}/}the inner${ROOT_DIR}is treated as a glob pattern, so any glob metacharacter in the repo path would alter the stripped prefix. Quoting it forces a literal match.♻️ Proposed fix
local rel - rel="${path#${ROOT_DIR}/}" + rel="${path#"${ROOT_DIR}"/}" echo "${rel}" | awk -F'/build/outputs/' '{print $1}'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package-sizes-report.sh` around lines 23 - 28, In module_name_from_path, the parameter expansion uses ${path#${ROOT_DIR}/} which treats ${ROOT_DIR} as a glob; change it to use a quoted pattern so the prefix is matched literally: assign rel using the quoted expansion rel="${path#"$ROOT_DIR"/}" (keep the surrounding double quotes for the assignment) to prevent globbing and satisfy ShellCheck SC2295.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/package-sizes.yml:
- Around line 14-18: Remove the unnecessary full-history checkout options:
delete the explicit ref: ${{ github.ref }} line and remove or change
fetch-depth: 0 to the default shallow clone (omit fetch-depth or set
fetch-depth: 1) in the actions/checkout@v4 step so the workflow uses a faster
shallow clone for the package-sizes job.
In `@package-sizes-report.sh`:
- Around line 18-21: The to_kb function currently interpolates ${bytes} directly
into the awk program which is unsafe; change it to pass bytes into awk via the
-v option (e.g., -v b="$bytes") and use b inside the awk script (printf "%.1f",
b/1024) so the numeric value is treated as data, not code, and avoids shell
interpolation issues when calling to_kb.
- Around line 23-28: In module_name_from_path, the parameter expansion uses
${path#${ROOT_DIR}/} which treats ${ROOT_DIR} as a glob; change it to use a
quoted pattern so the prefix is matched literally: assign rel using the quoted
expansion rel="${path#"$ROOT_DIR"/}" (keep the surrounding double quotes for the
assignment) to prevent globbing and satisfy ShellCheck SC2295.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de563b68-090b-472f-8adc-3c1f1a9d2679
📒 Files selected for processing (3)
.github/workflows/ci.yaml.github/workflows/package-sizes.ymlpackage-sizes-report.sh
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/ci.yaml
1 participant
JIRA Ticket
SDKS-4216 Calculate SDKs packages size
See also this wiki for more details...
Summary by CodeRabbit
New Features
Chores