ROSAENG-61040 | refactor: remove github.com/dustin/go-humanize#3414
ROSAENG-61040 | refactor: remove github.com/dustin/go-humanize#3414markirish wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (16)
📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe download helper removes Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (12 passed)
✨ Finishing Touches🧪 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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: markirish The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pkg/helper/download/download.go (1)
169-181: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the formatting constants.
The
10literals encode the rounding scale, threshold, and decimal precision. Extract named constants so the display contract is explicit and less error-prone.As per coding guidelines, avoid magic numbers; extract named constants when the value matters to behavior or readability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/helper/download/download.go` around lines 169 - 181, In the byte-formatting logic, replace the repeated literal 10 values used for rounding scale, threshold comparison, and one-decimal precision with clearly named constants. Update the condition and formatting expression in the surrounding formatting function while preserving the existing display behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/helper/download/download.go`:
- Around line 178-183: Update the byte-formatting logic around val and suffix to
assign the rounded one-decimal value from math.Round(val*10)/10, then use that
rounded value in both the one-decimal and zero-decimal format branches. Add a
regression test covering the exact halfway input 1250, expecting “1.3 kB”.
- Around line 113-114: Update the timeout error return in the download flow to
wrap the existing non-nil err with %w while preserving the current friendly
timeout message and URL context. Ensure callers can inspect the underlying
network error via errors.Is or errors.As.
---
Nitpick comments:
In `@pkg/helper/download/download.go`:
- Around line 169-181: In the byte-formatting logic, replace the repeated
literal 10 values used for rounding scale, threshold comparison, and one-decimal
precision with clearly named constants. Update the condition and formatting
expression in the surrounding formatting function while preserving the existing
display 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a47bf21-f041-4f55-9223-d507ebf0d690
⛔ Files ignored due to path filters (16)
go.sumis excluded by!**/*.sumvendor/github.com/dustin/go-humanize/.travis.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/README.markdownis excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/big.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/bigbytes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/bytes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/comma.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/commaf.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/ftoa.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/humanize.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/number.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/ordinals.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/si.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/dustin/go-humanize/times.gois excluded by!vendor/**,!**/vendor/**vendor/modules.txtis excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (3)
go.modpkg/helper/download/download.gopkg/helper/download/readable_bytes_test.go
💤 Files with no reviewable changes (1)
- go.mod
|
Lets see how my prompt works: @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR metadata and changed files =='
gh pr view 3414 --repo openshift/rosa --json number,url,title,baseRefName,headRefName,commits,files --jq '{number,url,title,base:.baseRefName,head:.headRefName,commits:[.commits[]|{oid:.oid,messageHeadline:.messageHeadline}],files:[.files[]|{path:.path,additions:.additions,deletions:.deletions}]}'
echo '== Diff against PR base =='
git diff --no-ext-diff --unified=80 "$(git merge-base HEAD origin/$(gh pr view 3414 --repo openshift/rosa --json baseRefName --jq .baseRefName))" HEAD -- go.mod go.sum pkg/helper/download/download.go pkg/helper/download/readable_bytes_test.go
echo '== Current module references and formatter call sites =='
rg -n -C 4 'github\.com/dustin/go-humanize|humanize\.Bytes|readableBytes\s*\(' go.mod go.sum pkg/helper/download
echo '== Go module version and formatting implementation context =='
rg -n -C 3 '^go\s+[0-9]+\.[0-9]+' go.mod
ast-grep outline pkg/helper/download/download.go --items all
sed -n '1,280p' pkg/helper/download/download.go
sed -n '1,240p' pkg/helper/download/readable_bytes_test.go
echo '== Introduction history for old module and call site =='
git log --all --reverse --format='%H%x09%s' -S'github.com/dustin/go-humanize' -- go.mod
git log --all --reverse --format='%H%x09%s' -S'humanize.Bytes' -- pkg/helper/download/download.go
echo '== Introduction commit details (first matching commit) =='
intro="$(git log --all --reverse --format='%H' -S'github.com/dustin/go-humanize' -- go.mod | head -n1)"
if [ -n "$intro" ]; then
git show --no-ext-diff --format=fuller --stat "$intro"
git show --no-ext-diff --format= --unified=30 "$intro" -- go.mod go.sum pkg/helper/download/download.go
fiLength of output: 43316 🤖 web_search executed:
Length of output: 6571 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/master)"
intro="a326f9842515a35186631e877cd7d0ac6c674fa3"
echo '== Introduction commit metadata and message =='
git show --no-ext-diff --no-patch --format=fuller "$intro"
git show --no-ext-diff --format= --unified=40 "$intro" -- go.mod go.sum pkg/helper/download/download.go
echo '== GitHub PRs associated with introduction commit =='
gh api -H 'Accept: application/vnd.github+json' \
"/repos/openshift/rosa/commits/$intro/pulls" \
--jq '.[] | {number,title,url:.html_url,body}'
echo '== Old v1.0.1 formatter implementation from PR base vendor tree =='
git show "$base:vendor/github.com/dustin/go-humanize/bytes.go" | sed -n '1,240p'
echo '== Current PR vendor/module cleanup verification =='
printf 'go.mod/go.sum module matches: '
if rg -n 'github\.com/dustin/go-humanize' go.mod go.sum; then false; else echo 'none'; fi
printf 'vendored path exists: '
if test -e vendor/github.com/dustin/go-humanize; then echo 'yes'; else echo 'no'; fi
printf 'vendor/modules.txt match: '
if rg -n 'github\.com/dustin/go-humanize' vendor/modules.txt; then false; else echo 'none'; fi
echo '== Compare formatter source behavior over representative edge cases =='
python3 - <<'PY'
import math
def old(s):
sizes = ["B", "kB", "MB", "GB", "TB", "PB", "EB"]
if s < 10:
return f"{s} B"
e = math.floor(math.log(float(s), 1000))
suffix = sizes[int(e)]
val = float(s) / math.pow(1000, math.floor(e))
return (f"{val:.1f} {suffix}" if val < 10 else f"{val:.0f} {suffix}")
def new(b):
unit = 1000
if b < unit:
return f"{b} B"
div, exp = unit, 0
n = b // unit
while n >= unit:
div *= unit
exp += 1
n //= unit
val = float(b) / float(div)
suffix = ["kB", "MB", "GB", "TB", "PB", "EB"][exp]
return f"{val:.1f} {suffix}" if round(val * 10) / 10 < 10 else f"{val:.0f} {suffix}"
cases = [
0, 1, 9, 10, 999, 1000, 9999, 10000,
999999, 1000000, 9999999, 10000000,
999999999, 1000000000, (2**64)-1
]
for value in cases:
print(f"{value}: old={old(value)!r} new={new(value)!r} {'OK' if old(value) == new(value) else 'DIFF'}")
PYLength of output: 33847
Paste-ready evaluation note:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== PR metadata and changed files ==='
gh pr view 3414 --repo openshift/rosa --json number,title,baseRefName,headRefName,commits,files --jq '{number,title,base:.baseRefName,head:.headRefName,commits:[.commits[]|{oid:.oid,messageHeadline:.messageHeadline}],files:[.files[]|{path:.path,additions:.additions,deletions:.deletions}]}'
echo '=== PR diff ==='
gh pr diff 3414 --repo openshift/rosa -- go.mod go.sum pkg/helper/download/download.go pkg/helper/download/readable_bytes_test.go
echo '=== Relevant source outline ==='
ast-grep outline pkg/helper/download/download.go --items all || true
echo '=== Relevant source and module entries ==='
sed -n '1,280p' pkg/helper/download/download.go
printf '\n--- go.mod occurrences ---\n'
rg -n -C2 'dustin/go-humanize|go-humanize' go.mod go.sum || true
printf '\n--- repository usages ---\n'
rg -n -C3 'humanize\.Bytes|go-humanize|readableBytes\s*\(' -g '*.go' -g 'go.mod' -g 'go.sum' .
echo '=== Addition provenance ==='
git log --all --format='%H%x09%s' -S'github.com/dustin/go-humanize' -- go.mod go.sum pkg/helper/download/download.go | head -20
git log --all --format='%H%x09%s' -S'humanize.Bytes' -- pkg/helper/download/download.go | head -20
echo '=== Commit details for adding commit(s) ==='
for commit in $(git log --all --format='%H' -S'github.com/dustin/go-humanize' -- go.mod go.sum pkg/helper/download/download.go | head -5); do
echo "--- $commit ---"
git show --no-ext-diff --format=fuller --stat "$commit"
git show --no-ext-diff --format=fuller "$commit" -- go.mod go.sum pkg/helper/download/download.go | sed -n '1,280p'
done
echo '=== Go version ==='
rg -n -C1 '^go [0-9.]+' go.modLength of output: 3926 🤖 web_search executed:
Length of output: 3940 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/rosa"
pr="3414"
head="7b244957a98a24b3b0a03cada2f17e3a0eb0e4cc"
echo '=== Full PR patch ==='
gh api -H 'Accept: application/vnd.github.patch' "/repos/${repo}/pulls/${pr}" > /tmp/pr-3414.patch
sed -n '1,900p' /tmp/pr-3414.patch
echo '=== Current replacement and tests ==='
ast-grep outline pkg/helper/download/download.go --items all || true
sed -n '1,300p' pkg/helper/download/download.go
sed -n '1,240p' pkg/helper/download/readable_bytes_test.go
echo '=== Residual dependency / formatter references in PR head ==='
rg -n -C3 'github\.com/dustin/go-humanize|humanize\.Bytes|readableBytes\s*\(' \
-g '*.go' -g 'go.mod' -g 'go.sum' -g 'modules.txt' . || true
echo '=== Module metadata and vendoring cleanup ==='
rg -n -C2 'dustin/go-humanize|go-humanize' go.mod go.sum vendor/modules.txt || true
rg -n -C1 '^go [0-9.]+' go.mod
echo '=== Removed upstream implementation at PR base ==='
base=$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq .baseRefOid)
gh api "/repos/${repo}/contents/vendor/github.com/dustin/go-humanize/bytes.go?ref=${base}" \
--jq .content | tr -d '\n' | base64 -d || true
echo '=== Local history candidates ==='
git log --all --format='%H%x09%s' -S'github.com/dustin/go-humanize' -- go.mod go.sum pkg/helper/download/download.go | head -30 || true
git log --all --format='%H%x09%s' -S'humanize.Bytes' -- pkg/helper/download/download.go | head -30 || true
echo '=== GitHub commit search candidates (addition) ==='
gh api -H 'Accept: application/vnd.github+json' \
"/search/commits?q=repo:${repo}+%22github.com/dustin/go-humanize%22" \
--jq '.items[] | [.sha, .commit.message, .html_url] | `@tsv`' | head -30 || true
echo '=== Repository commit history for affected files ==='
for path in go.mod pkg/helper/download/download.go; do
echo "--- ${path} ---"
gh api "/repos/${repo}/commits?path=${path}&per_page=100" \
--jq '.[] | [.sha, .commit.message, .html_url] | `@tsv`' | head -100 || true
done
echo '=== Upstream GitHub metadata / advisories ==='
gh api /repos/dustin/go-humanize --jq '{archived,disabled,license:(.license.spdx_id),updated_at,pushed_at,default_branch,security_and_analysis}' || true
gh api '/repos/dustin/go-humanize/security-advisories?per_page=100' --jq '[.[] | {ghsa_id,state,severity,published_at,withdrawn_at}]' || trueLength of output: 71873
PR description evaluation note: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== PR metadata and changed files =='
gh pr view 3414 --repo openshift/rosa --json number,title,baseRefName,headRefName,commits,files \
--jq '{number,title,baseRefName,headRefName,commits:[.commits[]|{oid,messageHeadline,authoredDate}],files:[.files[]|{path,additions,deletions}]}'
printf '%s\n' '== Current changed Go/package structure =='
ast-grep outline pkg/helper/download/download.go --items all
if [ -f pkg/helper/download/readable_bytes_test.go ]; then
ast-grep outline pkg/helper/download/readable_bytes_test.go --items all
fi
printf '%s\n' '== Relevant implementation and tests =='
rg -n -C 12 'readableBytes|humanize\.Bytes|WriteCounter|Downloaded' pkg/helper/download go.mod go.sum || true
printf '%s\n' '== All remaining module/import references =='
rg -n -C 3 'github\.com/dustin/go-humanize|go-humanize|humanize\.' . \
-g '!vendor/**' -g '!**/.git/**' || true
printf '%s\n' '== Dependency metadata =='
grep -n -C 2 'github.com/dustin/go-humanize' go.mod go.sum || true
go_version=$(awk '/^go [0-9]+\.[0-9]+/{print $2; exit}' go.mod)
printf 'Declared Go version: %s\n' "${go_version:-not found}"
printf '%s\n' '== Introduction/provenance of old module =='
git log --all --reverse -S'github.com/dustin/go-humanize' --format='%H%x09%ad%x09%s' --date=short -- go.mod go.sum
git log --all --reverse -S'humanize.Bytes' --format='%H%x09%ad%x09%s' --date=short -- pkg/helper/download/download.goLength of output: 12061 🤖 web_search executed:
Length of output: 5077 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Upstream repository metadata and README =='
gh api repos/dustin/go-humanize --jq '{full_name,archived,disabled,license:(.license.spdx_id // null),default_branch,updated_at,html_url}'
gh api repos/dustin/go-humanize/readme -H 'Accept: application/vnd.github.raw+json' | sed -n '1,240p'
printf '%s\n' '== GitHub security advisories for the removed module =='
gh api -X GET /advisories \
-f ecosystem=go \
-f affects='github.com/dustin/go-humanize' \
--paginate \
--jq '.[] | {ghsa_id,cve_id,summary,severity,published_at,withdrawn_at,html_url}' || trueLength of output: 4135 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Current replacement and its complete test cases =='
sed -n '145,220p' pkg/helper/download/download.go
sed -n '1,120p' pkg/helper/download/readable_bytes_test.go
printf '%s\n' '== Original formatter at the PR base =='
git show '7b244957a98a24b3b0a03cada2f17e3a0eb0e4cc^:vendor/github.com/dustin/go-humanize/bytes.go' | sed -n '1,240p'
printf '%s\n' '== Introduction and immediate dependency-fix patches =='
git show --format=fuller --find-renames --find-copies --stat --patch \
a326f9842515a35186631e877cd7d0ac6c674fa3 \
ee7556e686b87f3a29c719234b0e7373f81f68e7 \
-- go.mod go.sum vendor/modules.txt pkg/helper/download/download.go \
| sed -n '1,360p'Length of output: 35967
Paste-ready PR evaluation note: |
7b24495 to
3a5eeb6
Compare
@coderabbitai Can you confirm that the code now has parity with the old |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/rosa"
pr="3414"
base="$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq .baseRefOid)"
echo '== PR head and base =='
gh pr view "$pr" --repo "$repo" --json headRefOid,baseRefOid --jq '{head:.headRefOid,base:.baseRefOid}'
echo '== Current formatter and its tests =='
ast-grep outline pkg/helper/download/download.go --items all
sed -n '150,215p' pkg/helper/download/download.go
sed -n '1,180p' pkg/helper/download/readable_bytes_test.go
echo '== Original go-humanize v1.0.1 formatter from PR base =='
git show "${base}:vendor/github.com/dustin/go-humanize/bytes.go" | sed -n '1,220p'
echo '== Current call sites =='
rg -n -C 3 'readableBytes\s*\(' pkg/helper/download
echo '== Source-level comparison of precision predicates =='
rg -n -C 4 'func readableBytes|math\.(Log|Floor|Pow|Round)|%\.0f|%\.1f' \
pkg/helper/download/download.go
git show "${base}:vendor/github.com/dustin/go-humanize/bytes.go" \
| rg -n -C 4 'func Bytes|math\.(Log|Floor|Pow|Round)|%\.0f|%\.1f'Length of output: 9162
The earlier finding’s direction was reversed: the old formatter produced no decimal once the rounded value was at least 10, while
This follows directly from the old |
Replace humanize.Bytes with a ~15-line readableBytes helper using SI units (base 1000) matching the original behavior.
3a5eeb6 to
afbda87
Compare
|
@coderabbitai Ok, check one more time, parity should exist now |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/rosa"
pr="3414"
base="$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq -r .baseRefOid)"
head="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq -r .headRefOid)"
printf '== Revisions ==\nbase=%s\nhead=%s\n' "$base" "$head"
printf '\n== Updated helper and tests ==\n'
ast-grep outline pkg/helper/download/download.go --items all
sed -n '155,220p' pkg/helper/download/download.go
if [ -f pkg/helper/download/readable_bytes_test.go ]; then
sed -n '1,180p' pkg/helper/download/readable_bytes_test.go
fi
printf '\n== Removed formatter at PR base ==\n'
git show "${base}:vendor/github.com/dustin/go-humanize/bytes.go" | sed -n '1,240p'
printf '\n== Relevant PR changes ==\n'
git diff --no-ext-diff --unified=30 "$base" "$head" -- \
pkg/helper/download/download.go pkg/helper/download/readable_bytes_test.go
printf '\n== Behavioral comparison model (old vs updated logic) ==\n'
python3 - <<'PY'
import math
import random
MAX = (1 << 64) - 1
units = ["B", "kB", "MB", "GB", "TB", "PB", "EB"]
# Model of github.com/dustin/go-humanize v1.0.1 Bytes.
def old(s):
if s < 10:
return f"{s} B"
e = math.floor(math.log(float(s), 1000))
suffix = units[int(e)]
val = float(s) / math.pow(1000, math.floor(e))
return f"{val:.1f} {suffix}" if val < 10 else f"{val:.0f} {suffix}"
# Model of the current PR helper: unit choice from integer division;
# precision predicate must match the checked-in source.
def current(b):
unit = 1000
if b < unit:
return f"{b} B"
div, exp = unit, 0
n = b // unit
while n >= unit:
div *= unit
exp += 1
n //= unit
value = float(b) / float(div)
suffix = units[exp + 1]
return f"{value:.1f} {suffix}" if value < 10 else f"{value:.0f} {suffix}"
cases = set(range(0, 11000))
for exponent in range(1, 7):
base = 1000 ** exponent
for offset in range(-20, 21):
if 0 <= base + offset <= MAX:
cases.add(base + offset)
# Values around rounding from 9.9 to 10.0 in each unit.
for offset in range(-20, 21):
value = 10 * base + offset
if value <= MAX:
cases.add(value)
for _ in range(20000):
cases.add(random.Random(3414 + _).randrange(MAX + 1))
cases.add(MAX)
diffs = [(value, old(value), current(value)) for value in sorted(cases) if old(value) != current(value)]
print(f"cases_checked={len(cases)}")
print(f"differences={len(diffs)}")
for value, before, after in diffs[:20]:
print(f"DIFF {value}: old={before!r} current={after!r}")
if diffs:
raise SystemExit(1)
PYLength of output: 2949 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="openshift/rosa"
pr="3414"
base="$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq -r .baseRefOid)"
head="$(gh pr view "$pr" --repo "$repo" --json headRefOid --jq -r .headRefOid)"
printf '== Revisions ==\nbase=%s\nhead=%s\n' "$base" "$head"
printf '\n== Updated helper and its tests ==\n'
sed -n '155,225p' pkg/helper/download/download.go
sed -n '1,180p' pkg/helper/download/readable_bytes_test.go
printf '\n== Original go-humanize formatter at PR base ==\n'
git show "${base}:vendor/github.com/dustin/go-humanize/bytes.go" | sed -n '1,220p'
printf '\n== Updated relevant diff ==\n'
git diff --no-ext-diff --unified=30 "$base" "$head" -- \
pkg/helper/download/download.go pkg/helper/download/readable_bytes_test.go
printf '\n== Precision predicates ==\n'
rg -n -C 3 'func readableBytes|value < 10|val < 10|%\.0f|%\.1f' \
pkg/helper/download/download.go
git show "${base}:vendor/github.com/dustin/go-humanize/bytes.go" \
| rg -n -C 3 'func Bytes|value < 10|val < 10|%\.0f|%\.1f'
printf '\n== Behavioral comparison ==\n'
python3 - <<'PY'
import math
import random
MAX = (1 << 64) - 1
units = ["B", "kB", "MB", "GB", "TB", "PB", "EB"]
def old(s):
if s < 10:
return f"{s} B"
exponent = math.floor(math.log(float(s), 1000))
value = float(s) / math.pow(1000, exponent)
return f"{value:.1f} {units[int(exponent)]}" if value < 10 else f"{value:.0f} {units[int(exponent)]}"
def replacement(b):
if b < 1000:
return f"{b} B"
div, exp, n = 1000, 0, b // 1000
while n >= 1000:
div *= 1000
exp += 1
n //= 1000
value = float(b) / float(div)
suffix = units[exp + 1]
return f"{value:.1f} {suffix}" if value < 10 else f"{value:.0f} {suffix}"
cases = set(range(0, 11000))
for exponent in range(1, 7):
scale = 1000 ** exponent
for pivot in (scale, 10 * scale):
for offset in range(-100, 101):
if 0 <= pivot + offset <= MAX:
cases.add(pivot + offset)
rng = random.Random(3414)
for _ in range(20000):
cases.add(rng.randrange(MAX + 1))
cases.add(MAX)
diffs = [(v, old(v), replacement(v)) for v in sorted(cases) if old(v) != replacement(v)]
print(f"cases_checked={len(cases)}")
print(f"differences={len(diffs)}")
for v, previous, current in diffs[:20]:
print(f"DIFF {v}: old={previous!r} replacement={current!r}")
if diffs:
raise SystemExit(1)
PYLength of output: 2763 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '== Current helper =='
sed -n '160,215p' pkg/helper/download/download.go
printf '%s\n' '== Current parity tests =='
sed -n '1,160p' pkg/helper/download/readable_bytes_test.go
printf '%s\n' '== Original formatter from the parent revision =='
git show HEAD^:vendor/github.com/dustin/go-humanize/bytes.go | sed -n '1,180p'
printf '%s\n' '== Relevant precision branches =='
rg -n -C 4 'func readableBytes|value < 10|val < 10|%\.0f|%\.1f' pkg/helper/download/download.go
git show HEAD^:vendor/github.com/dustin/go-humanize/bytes.go \
| rg -n -C 4 'func Bytes|value < 10|val < 10|%\.0f|%\.1f'
printf '%s\n' '== Comparison across boundaries and deterministic random uint64 values =='
python3 - <<'PY'
import math
import random
limit = (1 << 64) - 1
names = ["B", "kB", "MB", "GB", "TB", "PB", "EB"]
def old(value):
if value < 10:
return f"{value} B"
exponent = math.floor(math.log(float(value), 1000))
formatted = float(value) / math.pow(1000, exponent)
if formatted < 10:
return f"{formatted:.1f} {names[int(exponent)]}"
return f"{formatted:.0f} {names[int(exponent)]}"
def new(value):
if value < 1000:
return f"{value} B"
divisor = 1000
exponent = 0
reduced = value // 1000
while reduced >= 1000:
divisor *= 1000
exponent += 1
reduced //= 1000
formatted = float(value) / float(divisor)
if formatted < 10:
return f"{formatted:.1f} {names[exponent + 1]}"
return f"{formatted:.0f} {names[exponent + 1]}"
values = set(range(11000))
for exponent in range(1, 7):
scale = 1000 ** exponent
for pivot in (scale, 10 * scale):
values.update(v for v in range(pivot - 100, pivot + 101) if 0 <= v <= limit)
rng = random.Random(3414)
values.update(rng.randrange(limit + 1) for _ in range(20000))
values.add(limit)
differences = [(v, old(v), new(v)) for v in sorted(values) if old(v) != new(v)]
print("cases_checked=", len(values))
print("differences=", len(differences))
for value, previous, replacement in differences[:20]:
print(f"{value}: old={previous!r}; replacement={replacement!r}")
if differences:
raise SystemExit(1)
PYLength of output: 9425
It uses the same SI base-1000 unit selection, rounding ( The reported Python differences were from a stale model of the previous integer-division implementation, not the current checked-in helper. The current source matches the original formatter’s algorithm. If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
@markirish: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
openshift-ci
Bot
added
the
needs-rebase
2 participants
PR Summary
Replace humanize.Bytes with a ~15-line readableBytes helper using SI units (base 1000) matching the original behavior.
Detailed Description of the Issue
Our ROSA CLI project currently imports several third-party libraries that are only utilized in a single file (e.g., exclusively within a test file or for a single validation function). Relying on entire external dependencies for isolated use cases unnecessarily increases our security attack surface, complicates dependency management, and bloats the project.
This PR is part of that initiative to remove
github.com/dustin/go-humanizeRelated Issues and PRs
Type of Change
Previous Behavior
Behavior After This Change
How to Test (Step-by-Step)
Preconditions
Test Steps
make testExpected Results
All tests pass
Proof of the Fix
Tests pass
Breaking Changes
Developer Verification Checklist
[JIRA-TICKET] | [TYPE]: <MESSAGE>.make install-hookshas been run in this clone.make testpasses.make lintpasses.make rosapasses.Summary by CodeRabbit
Summary
Bug Fixes
Tests