Replace re_path with path where practical#3627
Conversation
Test results 27 files 27 suites 44m 39s ⏱️ Results for commit ad5ba61. ♻️ This comment has been updated with latest results. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3627 +/- ##
==========================================
- Coverage 62.50% 62.50% -0.01%
==========================================
Files 611 611
Lines 45103 45100 -3
Branches 43 43
==========================================
- Hits 28193 28190 -3
Misses 16900 16900
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Simrayz
left a comment
There was a problem hiding this comment.
Some good work here 😄 Just have some questions/comments and a bug that should be addressed.
There was a problem hiding this comment.
You seem to have overlooked some re_paths, and you have not been explicit with param types in many places. Explicitness aids readability. Imagine having to debug on the first day back from a max-length vacation, with jet-lag and a hangover.
Also, if switching to path() is impossible, document why on the preceding line.
I haven't commented on every spot where a param type is missing. grep '<' **/urls.py | grep -v ':' might find them.
(.. and I see that Github's highlighter still highlights other lines than the ones I clicked on... sigh)
fe66e21 to
ad5ba61
Compare
|
Scope and purpose
Fixes #3548
This PR sticks to replacing re_path where it is practical and results in almost or fully identical behavior.
Contributor Checklist
Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to NAV can be found in the
Hacker's guide to NAV.
<major>.<minor>.x). For a new feature or other additions, it should be based onmaster.