-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
fix: primary keys need not to be integers, to serialize them is safer to use str() than int() #1806
base: master
Are you sure you want to change the base?
Conversation
…se str() than int()
Codecov Report
@@ Coverage Diff @@
## master #1806 +/- ##
=======================================
Coverage 78.29% 78.29%
=======================================
Files 72 72
Lines 8699 8699
=======================================
Hits 6811 6811
Misses 1888 1888
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Thank you for the PR.
Makes sense to me, lint is failing on CI
It actually seems that the fail on CI is due to codecov reporting that the patched lines are not covered by tests; it actually seems that all the lines in the modified file are not covered at all by any test. I ask you to approve and merge the patch, since it will be difficult for me to add tests for all the views. |
Is there reason why this isn't merged yet? |
@mapio I think Daniel's style on merging in this repo is bit different, he will approve all finalised prs and just before cutting a release he will rebase on master and merge. |
This patch fixes #1805.