Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
✨ v3 (feature): add configuration support to c.SendFile() #3017
✨ v3 (feature): add configuration support to c.SendFile() #3017
Changes from 12 commits
6f4c253
954ab45
366dd56
e4b4b0f
52cdab4
d151a02
e8246f3
ee2d704
fa20ed8
b3673a0
4743fde
be7f0b1
b5bcd0c
011c7d0
129b89d
0750870
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Documentation for
sendfiles
field still needed.The
sendfiles
field lacks documentation. Proper documentation is crucial for maintainability and understanding the purpose of the field, especially for public or exported fields.+ // sendfiles stores configurations for handling ctx.SendFile operations. sendfiles []*sendFileStore
Committable suggestion
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.
It's available above the ctx.SendFile method
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.
After reviewing the provided content of
app.go
, it appears that there isn't a direct reference to thesendfiles
field in the vicinity of thectx.SendFile
method. It's possible there might be some confusion or miscommunication regarding the location of the documentation. Could you please double-check or clarify where the documentation for thesendfiles
field is located? If it's in a different part of the file or in another file, I can assist in locating and reviewing it.Check warning on line 129 in ctx.go
ctx.go#L129
Check warning on line 133 in ctx.go
ctx.go#L133
Check warning on line 141 in ctx.go
ctx.go#L141
Check warning on line 145 in ctx.go
ctx.go#L145
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.
I know this is very old line of code, but why are we removing the header?
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.
Shouldnt fasthttp itself be checling the header and not serving compressed content if Compress is disabled in fasthttp.FS ?
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.
I'm not sure but fasthttp also seems to add it to servceuncompressed strAcceptEncodinghttps://github.com/valyala/fasthttp/blob/c7576cc10cabfc9c993317a2d3f8355497bea156/fs.go#L55
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.