labath added inline comments.
================ Comment at: lldb/cmake/modules/LLDBConfig.cmake:43 + else() + set(default_enable_python ON) + endif() ---------------- labath wrote: > JDevlieghere wrote: > > mgorny wrote: > > > JDevlieghere wrote: > > > > mgorny wrote: > > > > > JDevlieghere wrote: > > > > > > mgorny wrote: > > > > > > > I know that the same thing is done above but… what's the point of > > > > > > > setting it to `ON` again? > > > > > > It's just setting the default to the old value. If the value was > > > > > > set, we should honor it, both if it was `OFF` or `ON`. > > > > > But it's already set to `ON` a few lines above. > > > > Do you mean the `DEFINED`? That just checks if the variable exists, not > > > > its value. > > > No, I mean you have: > > > > > > ``` > > > set(default_enable_python ON) > > > #... > > > if (DEFINED LLDB_DISABLE_PYTHON) > > > #... > > > else() > > > set(default_enable_python ON) > > > ``` > > > > > > This is superfluous since the default is already `ON`. > > Got it, thanks. Simplified the code. > Yeah, I added the curses stuff without too much optimization to keep the > debian bot working until the master restarts (so the configuration change > takes effect). I don't think we have any bots using LLDB_DISABLE_PYTHON, so > this probably wasn't even needed, but this all isn't very important -- the > code will be gone in a few days anyway... Btw, I've just removed the LLDB_DISABLE_CURSES blurb. If you don't know of any reason to keep LLDB_DISABLE_PYTHON python around (I don't) then I think that can go too... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71482/new/ https://reviews.llvm.org/D71482 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
