-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feature/28 local base urls #29
Conversation
@@ -7,6 +9,21 @@ | |||
from rest_framework.request import Request | |||
|
|||
|
|||
def is_local(host: str, url: str) -> bool: |
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.
Could we have a few unittests for only this method? I'm all in favor of integration tests but this feels complicated enough to test separately as well
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.
Tests are added
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.
Request for additional tests, but imho this isn't mandatory. A double-check from Steven is though
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.
Just a question regarding validate_host
, other than that it looks good. I would also like to see the unittests that Alex mentioned, but this could maybe be added in a separate PR if we want to get this released quickly
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #29 +/- ##
==========================================
+ Coverage 99.78% 99.80% +0.01%
==========================================
Files 14 15 +1
Lines 476 516 +40
==========================================
+ Hits 475 515 +40
Misses 1 1 ☔ View full report in Codecov by Sentry. |
828fff0
to
11e73b0
Compare
fixes #28