-
Notifications
You must be signed in to change notification settings - Fork 260
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
Fixed issue #314 - include information about view/serializer in warnings #327
Fixed issue #314 - include information about view/serializer in warnings #327
Conversation
Codecov Report
@@ Coverage Diff @@
## master #327 +/- ##
==========================================
- Coverage 98.22% 98.08% -0.14%
==========================================
Files 53 54 +1
Lines 4729 4919 +190
==========================================
+ Hits 4645 4825 +180
- Misses 84 94 +10
Continue to review full report at Codecov.
|
…in warnings. The main technique for doing this in deeply nested places from which we call `error()` or `warn()`, is to a stack of traces that get added as prefices to the message (rather than having to pass this context information through many layers of code). This also fixes a set of consistency issues with warning/error messages: - Always start message with `{serializer/view name}: ` if possible. This meant removing this info from the body of the message where we are now adding it as a prefix. - First sentence shouldn't have a capital letter (the message is printed after colon from the prefix) - Subsequent sentences should have a capital letter.
37f8b0b
to
52b1c88
Compare
i think the difference is very manageable.
which imho is a lot better. always having a full trace including the view is more consistent and may also help when the same serializer name is used in different apps. i could already see the next person asking for this 😄 thanks again! i hope you get that number down to zero fast. |
This is a replacement for #322, with changes as suggested in the review there, squashed to reduce noise in the history.
I ran this against my clients project. With #322, I was getting:
With this I'm getting:
There are a few more unique because there are a few more places where are we are adding the
MyViewSet:
prefix. The bigger change is that most of my messages now haveMyViewSet: MySerializer
instead of justMySerializer:
prefix.