Daniel Sahlberg wrote on Wed, Apr 20, 2022 at 13:20:15 +0200: > Den ons 20 apr. 2022 kl 05:57 skrev Branko Čibej <br...@apache.org>: > > > On 18.04.2022 19:46, Nathan Hartman wrote: > > > On Sun, Apr 17, 2022 at 9:30 AM <danie...@apache.org> wrote: > > >> Author: danielsh > > >> Date: Sun Apr 17 13:30:40 2022 > > >> New Revision: 1899945 > > >> > > >> URL: http://svn.apache.org/viewvc?rev=1899945&view=rev > > >> Log: > > >> * subversion/tests/cmdline/__init__.py > > >> (): Rewrite a comment. > > >> > > >> Modified: > > >> subversion/trunk/subversion/tests/cmdline/svntest/__init__.py > > >> > > >> Modified: subversion/trunk/subversion/tests/cmdline/svntest/__init__.py > > >> URL: > > http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svntest/__init__.py?rev=1899945&r1=1899944&r2=1899945&view=diff > > >> > > ============================================================================== > > >> --- subversion/trunk/subversion/tests/cmdline/svntest/__init__.py > > (original) > > >> +++ subversion/trunk/subversion/tests/cmdline/svntest/__init__.py Sun > > Apr 17 13:30:40 2022 > > >> @@ -18,8 +18,6 @@ > > >> # under the License. > > >> # > > >> > > >> -# any bozos that do "from svntest import *" should die. export nothing > > >> -# to the dumbasses. > > >> __all__ = [ ] > > >> > > >> import sys > > >> > > >> > > > > > > This removes the comment, rather than rewriting it as suggested in the > > > log. > > >
Sorry that wasn't clear. It does, however, explain how I approached that comment: I hadn't _set out_ to delete it, but to rewrite it with the content preserved and the rest removed. The result of the rewrite was the empty string. > > > I agree the comment should be rewritten. It was added (along with the > > > __all__ = []) in r951379, the log of which reads: "Protect against bad > > > python proggies." I couldn't find other contextual information about > > > it, but I suppose any comment to that effect would be helpful? > > > > > > 'from X import *' is considered bad practice in Python, for various > > reasons. > > > > I'm not sure why the comment had to be removed, unless it's a case of > > overly sensitive political correctness. > > > > I disagree with "overly sensitive". There are other ways to convey the same > message that are probably equally effective in educating the reader. I > wouldn't approve of that kind of language at $dayjob and I don't think it > belongs here either. > > What about: > > [[[ > # from X import * is bad practice in Python. export nothing to those using > it. > ]]] I don't think that's what the comment should say. The target audience of comments is someone who is bilingual in Python and English. Therefore, comments shouldn't simply be a _translation_ of the code from Python to English; rather, the English should say something _over and above_ what has already been said in Python. "Export nothing to callsites that star-import this module" is precisely what «__all__ = [ ]» means. In the context of a Python file, it's assumed knowledge, much like the meaning of #! on the first line of a Python file or an include guard in a *.h file. So, we don't need a comment that translates the assignment from Python to English for the reader's benefit. What we _might_ need is a comment that explains _why_ we set __all__ to an empty sequence. I say "might", because if assigning «__all__ = [ ]» is standard practice, then it can go without any comment at all, just like include guards don't get comments; but on the other hand, if assigning «__all__ = [ ]» is _not_ standard practice, then we do need to explain why we do that (but we can assume the reader knows what "that" is). Makes sense? Daniel P.S. «__all__» is documented at <https://docs.python.org/3/genindex-_.html>.
