| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
This is the second item from the action items proposed by @terryjreedy (#63882 (comment))
|
Sorry, something went wrong.
|
@terryjreedy (I can’t request a review in any other way) |
Sorry, something went wrong.
There was a problem hiding this comment.
I think all of them are included. Looks good to me!
Sorry, something went wrong.
|
Why are we using ## instead of a simple # for commenting out the test? Is there any plan of actually implementing those tests in the future? can we add some # TODO as well so that we can find them easily in the future? |
Sorry, something went wrong.
|
@picnixz This was requested by Terry in his comment on the issue. I guess it is easier to find them? |
Sorry, something went wrong.
|
I think I'll me comfortable with an explicit "TODO" since we also used # CRASH for the test C API to make them easier to find (with a ##, we need to filter out the results but with # TODO, we know that's a TODO). Ok, I've seen that #63882 (comment) is actually the comment that explained the different steps to do (it was hidden by GH). So there's a plan to implement something. If the tests are meant to be implemented in a short future, ## is fine, but if it's not, then maybe # TODO is better. |
Sorry, something went wrong.
There was a problem hiding this comment.
Double hashes, '##', are an old standard for (temporarily) commenting out code with something other than normal code comments. I added a "# TODO" comment.
Sorry, something went wrong.
There was a problem hiding this comment.
Make the first 3 tests 1 line as with all the others.
Sorry, something went wrong.
|
These stubs have been present for 25 years, since Python 2.0! I'd rather see tests added rather than commented out. If not added, let's delete them otherwise I can see these being around until 2050 🙃 Quoting @voidspace:
https://opensource.com/article/17/5/30-best-practices-software-development-and-testing |
Sorry, something went wrong.
|
There is actually already a patch that would add some of them: https://bugs.python.org/file33258/issue19683.patch Tough I am not sure who submitted it. |
Sorry, something went wrong.
|
Maybe checking the old bpo page helps? |
Sorry, something went wrong.
There was a problem hiding this comment.
Some of these tests have been implemented. Surprisingly not a merge conflict.
Sorry, something went wrong.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
|
Thank you for the patch. Commenting the tests out was the right thing while there was no replacement, but they are implemented now in #156677, so this is no longer needed. The ## def testX(self): pass form would also have to be reconciled with the tests which have been implemented since January 2025, as noted in the last review. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Given the absence of immediate replacements, this was considered better than deletion.