Interesting lesson? Last August, a contributor wanted to fix something in #django-money's contrib.django_rest_framework. The PR had no tests, a few people mentioned that they'd like to have it merged.
So 9 months later, a maintainer (eh me), gives up and helps to write a few tests and get this thing merged. But no one with acclaimed DRF skillz had reviewed it.
Afterwards, it broke DRF integration in two ways: Two great new contributors came by, fixed it w/ tests!
(1) We can define a serializer without reference to a Django Model, even with a MoneyField. This broke the assumption that self.parent.Meta.model._meta was fine.
(2) A field can be renamed from its model source. That broke the assumption that self.field_name is defined on the model -- instead self.source was the right one.
Of course I shouldn't really review this, I can't. But it's a .contrib package, so who really is? 🤷
So I'm not sure what the lesson really is here, but maybe you'd care to comment?
Don't mind that you (maintainer) can't really review something in .contrib. Community can click the "Watch" button on the repo if they want to help out here. Or we can use CODEOWNERS even.
Don't worry too much: If the .contrib package is used and it breaks, then nice people will fix it.
Don't be overly confident in tests. This particular thing actually had many good test cases.
@benjaoming Seems like a success to me. In the bazaar (of users at least) every code branch is covered somewhere, and a breakage is picked up and fixed, and the package is improved.
As a user I need to make sure I have tests on the bits I’m using. If a breakage comes in I can pin the old version while we work out a fix.
Seems like the only alternative to that is to have infallible maintainers, which we don’t 😜
@carlton yes! I love that it was fixed... worst case, someone would talk about a regression, rolling back the change, som struggle between users with different needs would ensue with the maintainer as the unwilling mediator etc. Lots of unproductive scenarios possible.
I could wonder if these contrib packages could need a definition added. Something like "this package provides important functionality outside of the maintainer's interest/expertise. Please help review changes".
Add comment