Skip to content
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

Deprecated ErrStatusRequestEntityTooLarge #2426

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

siyul-park
Copy link
Contributor

  • Deprecated ErrStatusRequestEntityTooLarge and Add ErrStatusRequestEntityTooLarge

@siyul-park siyul-park changed the title feat: Deprecated ErrStatusRequestEntityTooLarge Deprecated ErrStatusRequestEntityTooLarge Mar 29, 2023
@codecov
Copy link

codecov bot commented Mar 29, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (f22ba67) 92.87% compared to head (da640e8) 92.87%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2426   +/-   ##
=======================================
  Coverage   92.87%   92.87%           
=======================================
  Files          39       39           
  Lines        4519     4519           
=======================================
  Hits         4197     4197           
  Misses        234      234           
  Partials       88       88           
Impacted Files Coverage Δ
echo.go 95.47% <ø> (ø)
middleware/body_limit.go 96.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@aldas
Copy link
Contributor

aldas commented Apr 15, 2023

This is probably best to leave as is (until v5). Adding/exposing another public API variable, and now having to maintain two exactly same errors (with different name), has longer effect than having little bit unfortunately named variable.

@lammel what do you think?

@lammel
Copy link
Contributor

lammel commented Apr 17, 2023

This section was revised in PR #2277 . Seems like we overlooked a chance to correct the naming there.

Not sure how often the old const is used in the wild (for me it's about 1 of ten projects)
I'm not completely against adding a deprecation in echo v4, but v5 is around the corner and deprecating it would mean we could remove the wrong value with v5 already then, which seems a little to sporty. But we don't have to of course.

As it is a pretty minor change I vote for approving the change and get the errors clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants