-
Notifications
You must be signed in to change notification settings - Fork 0
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
17 pragma based router #19
Conversation
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.
Checked out the PR and wanted to drop some quick thoughts. First off, love the router revamp and the use of pragma for routing – neat and flexible without being bloated.
The handling of dev vs. production environments is a smart move for maintainability. Might be worth adding a few comments or brief docs for clarity, especially for us not knee-deep in the details.
Lastly, while the optimizations and refactoring are top-notch, keeping an eye on test coverage would be cool, just to ensure no unintended breakages sneak through
Thank you for taking a look @paulwilke ! I've added this one to have a reminder on having a how-to for deploys And for detecting regressions and gaining trust improve that coverage is a must. Seeing that |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19 +/- ##
=========================================
+ Coverage 0.71% 4.93% +4.21%
=========================================
Files 25 25
Lines 3070 3123 +53
=========================================
+ Hits 22 154 +132
+ Misses 3048 2969 -79 ☔ View full report in Codecov by Sentry. |
The builder now generates the methods that defines routes in the presenters' class side.
Feels quite comfortable and makes the a CRUD fully usable by default just after being created without any additional change.