[libcxx-commits] [PATCH] D154987: [lit] Implement PYTHON directive and config.prologue

Tue Aug 8 03:15:04 PDT 2023

jhenderson added a comment.

In D154987#4559192 <https://reviews.llvm.org/D154987#4559192>, @jdenny wrote:

> Thanks for the additional comments from everyone.
>
> In D154987#4508046 <https://reviews.llvm.org/D154987#4508046>, @ldionne wrote:
>
>> It looks like there's no reliable way to test what I was trying to test, and that test isn't *that* important anyway. I'll remove it since it was broken in the first place, so just rebase your patch. Sorry for the trouble.
>
> No worries, and thanks for the fix.  I finally got around to the rebase.
>
> In D154987#4539229 <https://reviews.llvm.org/D154987#4539229>, @awarzynski wrote:
>
>> In D154987#4531662 <https://reviews.llvm.org/D154987#4531662>, @jhenderson wrote:
>>
>>> I think it would be worth outlining in this patch why simply calling an external python script (which could actually be embedded within the file) is insufficient for the motivating case.
>>
>> +1
>
> I just extended `TestingGuide.rst`.  Let me know if it's not what you are looking for.

Thanks. In my case at least, I was just referring to enhancing the commit description or even just posting a review comment with the explanation. Enhancing the documentation works for me though.

================
Comment at: llvm/docs/TestingGuide.rst:348
+files. As usual in python, you can define such symbols in a python module (but
+see `config.prologue`_ below for an alternative approach if you need to do share
+them across an entire test directory). For example, if the module file
----------------

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154987/new/

https://reviews.llvm.org/D154987