labath accepted this revision. labath added a subscriber: aprantl. labath added a comment. This revision is now accepted and ready to land.
In D83425#2161255 <https://reviews.llvm.org/D83425#2161255>, @bbli wrote: > > Setting this no longer makes sense, as it will always be empty. Please > > remove that. Maybe also rename stdout_content to indicate it also contains > > stderr. Just plain "output" might suffice? > > Ok, deleted it and changed "stdout_content" to "combined_content". Thanks this looks good now. I presume you don't have commit access and need someone to commit this for you? >> this error result also doesn't make sense. It looks like you'll also need to >> update the usage in `getCompilerVersion` > > Ohh your right. I should have check whether other functions depended on > stderr stream. So checking the rest of the codebase, it seems the output of > `system` is also used in `TestDataFormatterSkipSummary.py`, but b/c it only > triggers on Macs, I didn't see a failure when I made the change. LOL, I wonder when was the last time anyone ran the lldb test suite against gcc on a mac. @aprantl, can we just delete the version check blurb in `TestDataFormatterSkipSummary.py`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83425/new/ https://reviews.llvm.org/D83425 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
