Skip to content

♻️ Refactor: replace appendLowerBytes with utilsbytes.UnsafeToLower#4468

Merged
ReneWerner87 merged 2 commits into
gofiber:mainfrom
ksw2000:refact-remove-appendLowerBytes
Jun 28, 2026
Merged

♻️ Refactor: replace appendLowerBytes with utilsbytes.UnsafeToLower#4468
ReneWerner87 merged 2 commits into
gofiber:mainfrom
ksw2000:refact-remove-appendLowerBytes

Conversation

@ksw2000

@ksw2000 ksw2000 commented Jun 28, 2026

Copy link
Copy Markdown
Member

Description

The appendLowerBytes function in Fiber can be replaced with utilsbytes.UnsafeToLower to improve maintainability

Type of change

  • Performance improvement (non-breaking change which improves efficiency)
  • Code consistency (non-breaking change which improves code reliability and robustness)

Checklist

Before you submit your pull request, please make sure you meet these requirements:

  • Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
  • Conducted a self-review of the code and provided comments for complex or critical parts.
  • Updated the documentation in the /docs/ directory for Fiber's documentation.
  • Added or updated unit tests to validate the effectiveness of the changes or new features.
  • Ensured that new and existing unit tests pass locally with the changes.
  • Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
  • Aimed for optimal performance with minimal allocations in the new code.
  • Provided benchmarks for the new code to analyze and improve upon.

Commit formatting

Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7c58fec3-2f89-4ef5-8fcd-3a0f8a33bb37

📥 Commits

Reviewing files that changed from the base of the PR and between fe33129 and 3924435.

📒 Files selected for processing (2)
  • ctx.go
  • path.go
💤 Files with no reviewable changes (1)
  • path.go

Walkthrough

Removes the local appendLowerBytes helper from path.go and replaces its usage in ctx.go's configDependentPaths with utilsbytes.UnsafeToLower for in-place lowercasing of detectionPath.

Changes

Detection path lowercasing refactor

Layer / File(s) Summary
Switch to utilsbytes.UnsafeToLower
ctx.go, path.go
Adds utilsbytes import alias; configDependentPaths now always copies c.path into c.detectionPath then conditionally lowercases in-place via utilsbytes.UnsafeToLower; removes the now-unused appendLowerBytes helper from path.go.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • gofiber/fiber#3353: Refactors DefaultCtx routing path handling using byte-slice path/detectionPath at the same code points modified here.
  • gofiber/fiber#4087: Also updates ctx.go to use utilsbytes.UnsafeToLower for case-insensitive path detection and refactors lowercasing logic in path.go.

Suggested labels

⚡️ Performance

Suggested reviewers

  • sixcolors
  • efectn
  • gaby

Poem

🐇 A helper once lived in path.go's den,
Lowercasing letters again and again.
Now UnsafeToLower does the trick in-place,
Less code, same result — a tidier space.
Hop hop, the rabbit approves this refrain! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main refactor and matches the changes in the patch.
Description check ✅ Passed The description covers purpose, type of change, checklist, and commit formatting, though it omits the issue reference and detailed changes section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jun 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.84%. Comparing base (fe33129) to head (3924435).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4468      +/-   ##
==========================================
- Coverage   92.89%   92.84%   -0.06%     
==========================================
  Files         138      138              
  Lines       13486    13474      -12     
==========================================
- Hits        12528    12510      -18     
- Misses        591      596       +5     
- Partials      367      368       +1     
Flag Coverage Δ
unittests 92.84% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ksw2000 ksw2000 marked this pull request as ready for review June 28, 2026 03:25
@ksw2000 ksw2000 requested a review from a team as a code owner June 28, 2026 03:25
@ReneWerner87

Copy link
Copy Markdown
Member

@ksw2000 can you share a before and after bench

@ReneWerner87

Copy link
Copy Markdown
Member

Or is it the same speed

@ksw2000

ksw2000 commented Jun 28, 2026

Copy link
Copy Markdown
Member Author

@ksw2000 can you share a before and after bench

I think the efficiency can be handled and guaranteed by github.com/gofiber/utils/v2/bytes

@ReneWerner87 ReneWerner87 merged commit a469916 into gofiber:main Jun 28, 2026
17 checks passed
@github-project-automation github-project-automation Bot moved this to Done in v3 Jun 28, 2026
@ReneWerner87 ReneWerner87 modified the milestones: v3, v3.4.0 Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants