gh-100450: Add sqlite.Row.__contains__ method#100457
Conversation
6ba3bdd to
5b089dd
Compare
December 23, 2022 11:58
erlend-aasland
left a comment
There was a problem hiding this comment.
I left some nitpicks. Also, please add tests.
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
|
@erlend-aasland Hmm, is this whole thing not wrong anyway? (See my comment in the issue thread.) |
Sorry, something went wrong.
I've added two doctests, aren't they enough? |
Sorry, something went wrong.
The doctests are there to make sure the examples in the docs actually work; it is not the place for unit tests. The unit tests (and regression tests, functional tests, etc.) all live in If I want to check the code coverage of the Also, I'm not sure the buildbots run doctests; IIRC, they only run the test suite, but I might be wrong about this. |
Sorry, something went wrong.
Thanks! BTW; I haven't decided if this is a good thing to add, or not. But lets keep that conversation on the issue. I'm going to mark this with do-not-merge until the discussion has landed. |
Sorry, something went wrong.
Should we open this to wider discussion on Discourse/Core development? |
Sorry, something went wrong.
|
Yeah, IMO it is worth it trying to gather more feedback on Discourse. |
Sorry, something went wrong.
I went ahead and added it :)
CC @erlend-aasland and @vsajip