-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: [#504] Route supports configure timeout #112
Conversation
github.com/goravel/goravel/issues/504 add WithContext
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.
Please merge your two PRs, one PR is fine. And add the global configuration logic.
i merged two PRs |
Important Review skippedAuto reviews are limited to specific labels. π·οΈ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Please update this PR according to the gin PR.
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.
Sorry, there are a lot of issues, please follow the goravel/gin PR to update this.
- Update go.mod, upgrade goravel/framework to the latest version;
- Remove the test case in
context_test.go
, just like what goravel/gin did; - Update
route.go::GlobalMiddleware
, just like what goravel/gin did; - Update
group_test.go
, the test cases should not pass now; - Rename
cors.go
tomiddleware_cors.go
, the same astls.go
; - Bebase your branch;
Thanks for your updates. I'm not sure if this PR is ready to be reviewed again, just notice, a few points are still not updated. eg: go.mod, rename file, etc. |
The merge-base changed after approval.
Unfortunately, CI still failed. |
You can ask me if you have any questions for this. |
Do I understand correctly that I need to update the mock files? |
Yes, you need to optimize the test flow, there are some unexpected mockConfig assertion. |
Hey @KlassnayaAfrodita Are you still working on this, please? If you feel it's difficult to fix the CI, you can add me to your fiber repo, I can solve it. |
I invited you to the repository, could you then describe how this problem is solved. current branch patch-1 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #112 +/- ##
==========================================
- Coverage 79.09% 76.05% -3.04%
==========================================
Files 10 13 +3
Lines 885 1040 +155
==========================================
+ Hits 700 791 +91
- Misses 159 214 +55
- Partials 26 35 +9 β View full report in Codecov by Sentry. |
github.com/goravel/goravel/issues/504
add WithContext
π Description
Closes goravel/goravel#504
@coderabbitai summary
β Checks