bpo-40823: Use loadTestsFromTestCase() iso. makeSuite() in sqlite3 tests#20538
bpo-40823: Use loadTestsFromTestCase() iso. makeSuite() in sqlite3 tests#20538berkerpeksag merged 6 commits into
Conversation
5742333 to
aaf8ec3
Compare
November 17, 2020 22:24
berkerpeksag
left a comment
There was a problem hiding this comment.
I don't have a strong objection to this, but if we are going to drop Check prefix, I'd prefer using test_* instead of test*.
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.
No problem. I'll rebase onto master and fix the naming. |
Sorry, something went wrong.
aaf8ec3 to
fbc3b24
Compare
January 6, 2021 19:33
Ref. issue 5846: unittest.makeSuite is deprecated
fbc3b24 to
0f3b5fd
Compare
January 6, 2021 19:34
|
PTAL, @berkerpeksag. Rebased onto master with snake case method naming. IMHO, it looks really nice now. |
Sorry, something went wrong.
|
I have made the requested changes; please review again |
Sorry, something went wrong.
|
Thanks for making the requested changes! @berkerpeksag: please review the changes made to this pull request. |
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.
test_cursor_description_ctes_multiple_columns => test_cursor_description_cte_multiple_columns
|
I have made the requested changes; please review again |
Sorry, something went wrong.
|
Thanks for making the requested changes! @berkerpeksag: please review the changes made to this pull request. |
Sorry, something went wrong.
|
GitHub messed up the encoding again! See #20538 (comment). I must remember no to use the suggestion feature for these files. We should normalise the encodings: |
Sorry, something went wrong.
|
The same thing happened in #20448, and we ended up just converting Lib/sqlite3/test/userfunctions.py to UTF-8 in the same PR. I'd say we convert the rest right away. Agreed, @berkerpeksag? |
Sorry, something went wrong.
I failed to see what's wrong here and #20448 has lots of comments. Why do you want to convert encodings of these files? |
Sorry, something went wrong.
Commit fd4f651 was auto-generated by GitHub from the "suggestion" I set up in #20538 (comment). As you notice from the diff, GitHub silently converted the file from ISO-8859 encoding to UTF-8 encoding (ignoring the |
Sorry, something went wrong.
|
Ah, I see now, thanks for the explanation! Personally, I prefer reverting the commit generated by GitHub and apply the suggested change manually. I don't like when companies try to impose their own practices (it doesn't mean they are bad practices) on a perfectly fine piece of code :) |
Sorry, something went wrong.
|
… or we can revert the encoding change and create an issue for normalising the encodings.
No problem. I'll open an issue for normalising the file encodings. Consistence FTW :) |
Sorry, something went wrong.
berkerpeksag
left a comment
There was a problem hiding this comment.
LGTM!
Sorry, something went wrong.
https://bugs.python.org/issue40823