jhenderson added a comment. In D74023#1949261 <https://reviews.llvm.org/D74023#1949261>, @HsiangKai wrote:
> In D74023#1948388 <https://reviews.llvm.org/D74023#1948388>, @MaskRay wrote: > > > The code generally looks good. For unittests, I think we can either make > > llvm-readobj -A canonical or the unittests canonical. If we decide to place > > tests on one place, we should delete most tests on the other side. > > > > My current preference is that we use more of unittests and leave the > > minimum to `test/llvm-readobj/ELF/{ARM,RISCV}/` > > > Agree. I will remove redundant tests in > test/tools/llvm-readobj/ELF/{ARM,RISCV}/. I also agree with testing primarily through the unit-tests where possible. However, there are probably aspects that still need to be tested via lit, namely code behaviour in ELFDumper.cpp, so just be slightly wary. High-level examples that might or might not be relevant for this patch are making sure that the output is printed correctly, and making sure errors/warnings can be threaded up to the final output. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74023/new/ https://reviews.llvm.org/D74023 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
