-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
docs: Improve ctx.Locals method description, godoc and example #3032
Conversation
WalkthroughThe changes enhance the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3032 +/- ##
==========================================
- Coverage 82.86% 82.78% -0.08%
==========================================
Files 115 115
Lines 8227 8227
==========================================
- Hits 6817 6811 -6
- Misses 1082 1086 +4
- Partials 328 330 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- ctx.go (2 hunks)
- docs/api/ctx.md (2 hunks)
Additional context used
LanguageTool
docs/api/ctx.md
[uncategorized] ~223-~223: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...urns a pointer to the Bind struct which contains all the methods to bind ...
[uncategorized] ~225-~225: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...he request/response data. For detailed information check the Bind documentati...
[uncategorized] ~257-~257: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...// []byte("user=john") }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~281-~281: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...// []byte("user=john") }) ``` :::info Returned value is only valid within the handler....
[style] ~341-~341: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ... information from a ClientHello message in order to guide application logic in the GetCerti...
[typographical] ~364-~364: Consider adding a comma here. (PLEASE_COMMA)
Context: ...text() *fasthttp.RequestCtx ``` :::info Please read the [Fasthttp Documentation](https...
[uncategorized] ~423-~423: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...oe") // "doe" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~541-~541: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ... if not exist // .. }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~549-~549: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2)
Context: ...icate that the client cache is now stale and the full response should be sent. When...
[uncategorized] ~581-~581: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...n") // "john" // .. }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~596-~596: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...ers() map[string][]string ``` :::info Returned value is only valid within the handler....
[uncategorized] ~624-~624: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...n") // "john" // .. }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~639-~639: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...ers() map[string][]string ``` :::info Returned value is only valid within the handler....
[uncategorized] ~692-~692: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ..."google.com" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~717-~717: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ..."google.com" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~1129-~1129: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...q=something" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~1182-~1182: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...first wildcard segment }) ``` :::info Returned value is only valid within the handler....
[grammar] ~1275-~1275: Did you mean “are” or “were”? (SENT_START_NNS_IS)
Context: ..." // ... }) ``` ## Queries Queries is a function that returns an object conta...
[uncategorized] ~1362-~1362: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...") // "nike" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~1430-~1430: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...s the Redirect reference. For detailed information check the Redirect doc...
[uncategorized] ~1449-~1449: Did you mean: “By default,”? (BY_DEFAULT_COMMA)
Context: ... data and sends atext/html
response. By defaultRender
uses the default [**Go Templat...
[style] ~1449-~1449: To make your writing clearer, consider a more direct alternative. (TAKE_A_LOOK)
Context: ...want to use another View engine, please take a look at our [Template middleware](h...
[grammar] ~1457-~1457: Please check the verb form. (SHE_LIVE)
Context: ....string) error ``` ## Request Request return the [*fasthttp.Request](https://godoc....
[style] ~1457-~1457: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...Request Request return the [*fasthttp.Request](https://godoc.org/github.com/valyala/f...
[grammar] ~1472-~1472: Please check the verb form. (SHE_LIVE)
Context: ...te("GET") }) ``` ## Response Response return the [*fasthttp.Response](https://godoc...
[style] ~1472-~1472: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...sponse Response return the [*fasthttp.Response](https://godoc.org/github.com/valyala/f...
[style] ~1494-~1494: This phrase is redundant. Consider using “outside”. (OUTSIDE_OF)
Context: ...x *fasthttp.RequestCtx) ``` It is used outside of the Fiber Handlers to reset the context...
[uncategorized] ~1498-~1498: There should be no space here. (EG_SPACE)
Context: ...y be helpful after overriding the path, i. e. an internal redirect. Note that handler...
[uncategorized] ~1498-~1498: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...t. Note that handlers might be executed again which could result in an infinite loop....
[uncategorized] ~1552-~1552: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ... return err } } ``` ## SaveFile Method is used to save any multipart file ...
[typographical] ~1623-~1623: Consider adding a comma here. (PLEASE_COMMA)
Context: ...http or https for TLS requests. :::info Please use [Config.EnableTrustedProxyCheck
](...
Markdownlint
docs/api/ctx.md
983-983: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1005-1005: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1189-1189: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1190-1190: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1368-1368: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1369-1369: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1392-1392: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1393-1393: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1394-1394: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1285-1285: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1286-1286: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1287-1287: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1288-1288: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1289-1289: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1297-1297: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1298-1298: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1299-1299: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1307-1307: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1308-1308: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1309-1309: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1310-1310: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1318-1318: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1319-1319: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1320-1320: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1328-1328: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1329-1329: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1330-1330: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1331-1331: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1332-1332: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1333-1333: Column: 1 (MD010, no-hard-tabs)
Hard tabs
168-168: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1188-1188: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1216-1216: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1446-1446: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1209-1209: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
1398-1398: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
Additional comments not posted (3)
docs/api/ctx.md (2)
948-951
: The updated documentation for theLocals
method is clear and informative.It effectively explains the scope and lifecycle of stored variables, including the automatic removal and closure of resources implementing
io.Closer
. The tip about using non-exported types as keys to avoid collisions is a valuable addition for best practices.
962-962
: The example code effectively demonstrates the use of non-exported types as keys and type assertions.This is a practical example that not only shows how to use the
Locals
method but also reinforces the documentation on preventing key collisions and ensuring type safety.Also applies to: 969-976
ctx.go (1)
Line range hint
862-880
: Clarify the behavior ofLocals
methods regardingio.Closer
.The documentation added to the
Locals
methods clearly explains the lifecycle of stored values, including the automatic removal and the invocation of theClose
method for values implementingio.Closer
. This is a crucial detail for proper resource management and helps prevent resource leaks.
|
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/api/ctx.md (2 hunks)
Additional context used
LanguageTool
docs/api/ctx.md
[uncategorized] ~225-~225: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...he request/response data. For detailed information check the Bind documentati...
[uncategorized] ~257-~257: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...// []byte("user=john") }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~281-~281: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...// []byte("user=john") }) ``` :::info Returned value is only valid within the handler....
[style] ~341-~341: Consider a shorter alternative to avoid wordiness. (IN_ORDER_TO_PREMIUM)
Context: ... information from a ClientHello message in order to guide application logic in the GetCerti...
[typographical] ~364-~364: Consider adding a comma here. (PLEASE_COMMA)
Context: ...text() *fasthttp.RequestCtx ``` :::info Please read the [Fasthttp Documentation](https...
[uncategorized] ~423-~423: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...oe") // "doe" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~541-~541: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ... if not exist // .. }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~549-~549: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2)
Context: ...icate that the client cache is now stale and the full response should be sent. When...
[uncategorized] ~581-~581: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...n") // "john" // .. }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~596-~596: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...ers() map[string][]string ``` :::info Returned value is only valid within the handler....
[uncategorized] ~624-~624: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...n") // "john" // .. }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~639-~639: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...ers() map[string][]string ``` :::info Returned value is only valid within the handler....
[uncategorized] ~692-~692: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ..."google.com" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~717-~717: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ..."google.com" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~1129-~1129: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...q=something" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~1136-~1136: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...](../#zero-allocation) ::: ## Params Method can be used to get the route parameters...
[uncategorized] ~1182-~1182: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...first wildcard segment }) ``` :::info Returned value is only valid within the handler....
[grammar] ~1275-~1275: Did you mean “are” or “were”? (SENT_START_NNS_IS)
Context: ..." // ... }) ``` ## Queries Queries is a function that returns an object conta...
[uncategorized] ~1362-~1362: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...") // "nike" // ... }) ``` :::info Returned value is only valid within the handler....
[uncategorized] ~1430-~1430: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...s the Redirect reference. For detailed information check the Redirect doc...
[uncategorized] ~1449-~1449: Did you mean: “By default,”? (BY_DEFAULT_COMMA)
Context: ... data and sends atext/html
response. By defaultRender
uses the default [**Go Templat...
[style] ~1449-~1449: To make your writing clearer, consider a more direct alternative. (TAKE_A_LOOK)
Context: ...want to use another View engine, please take a look at our [Template middleware](h...
[grammar] ~1457-~1457: Please check the verb form. (SHE_LIVE)
Context: ....string) error ``` ## Request Request return the [*fasthttp.Request](https://godoc....
[style] ~1457-~1457: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...Request Request return the [*fasthttp.Request](https://godoc.org/github.com/valyala/f...
[grammar] ~1472-~1472: Please check the verb form. (SHE_LIVE)
Context: ...te("GET") }) ``` ## Response Response return the [*fasthttp.Response](https://godoc...
[style] ~1472-~1472: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...sponse Response return the [*fasthttp.Response](https://godoc.org/github.com/valyala/f...
[style] ~1494-~1494: This phrase is redundant. Consider using “outside”. (OUTSIDE_OF)
Context: ...x *fasthttp.RequestCtx) ``` It is used outside of the Fiber Handlers to reset the context...
[uncategorized] ~1498-~1498: There should be no space here. (EG_SPACE)
Context: ...y be helpful after overriding the path, i. e. an internal redirect. Note that handler...
[uncategorized] ~1585-~1585: Possible missing article found. (AI_HYDRA_LEO_MISSING_THE)
Context: ...n err } }) ``` ## SaveFileToStorage Method is used to save any multipart file ...
[typographical] ~1623-~1623: Consider adding a comma here. (PLEASE_COMMA)
Context: ...http or https for TLS requests. :::info Please use [Config.EnableTrustedProxyCheck
](...
Markdownlint
docs/api/ctx.md
983-983: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1005-1005: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1189-1189: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1190-1190: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1368-1368: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1369-1369: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1392-1392: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1393-1393: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1394-1394: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces
1285-1285: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1286-1286: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1287-1287: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1288-1288: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1289-1289: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1297-1297: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1298-1298: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1299-1299: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1307-1307: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1308-1308: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1309-1309: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1310-1310: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1318-1318: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1319-1319: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1320-1320: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1328-1328: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1329-1329: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1330-1330: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1331-1331: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1332-1332: Column: 1 (MD010, no-hard-tabs)
Hard tabs
1333-1333: Column: 1 (MD010, no-hard-tabs)
Hard tabs
168-168: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1188-1188: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1216-1216: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1446-1446: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1209-1209: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
1398-1398: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
Additional comments not posted (2)
docs/api/ctx.md (2)
948-951
: Enhanced documentation forctx.Locals
clarifies lifecycle and type safety.The updated documentation clearly explains the lifecycle of stored variables and the importance of type assertions, which improves the understanding and correct usage of the
ctx.Locals
method.
960-960
: Updated example code demonstrates best practices for handling keys and type assertions.The example code effectively demonstrates the use of non-exported types for keys to avoid collisions and the proper way to perform type assertions. This is a valuable update for preventing common errors and ensuring data integrity.
Also applies to: 962-962, 967-967, 970-970, 975-976
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- ctx_interface_gen.go (1 hunks)
Additional comments not posted (1)
ctx_interface_gen.go (1)
177-180
: The documentation added to theLocals
method clearly explains the lifecycle of stored values and their cleanup process. However, consider a slight grammatical adjustment for clarity.- // All the values are removed from ctx after returning from the top - // RequestHandler. Additionally, Close method is called on each value - // implementing io.Closer before removing the value from ctx. + // All values are removed from ctx after returning from the top + // RequestHandler. Additionally, the Close method is called on each value + // that implements io.Closer before the value is removed from ctx.
Description
Improve the
ctx.Locals
documentation to clarify the behavior of stored variables and their lifecycle to avoid unintentional resource closures, and provide best practices with keys and values.Changes Introduced
io.Closer
interface, itsClose
method will be called before it's removed.Details
docs/api/ctx.md
andctx.go
Locals
method documentation.ctx.Locals
regarding variable removal andio.Closer
interface handling.These changes aim to prevent issues with unintentional resource closures, such as database connections, and enhance the clarity and safety of using
ctx.Locals
infiber
.