clayborg added a comment. In https://reviews.llvm.org/D50365#1191944, @labath wrote:
> I am not sure I'll have the resources to see this review through, so I'd > prefer to leave this to someone else. > > The thoughts I have had so far are: > > - the patch is very big and probably runs afoul of the "you shall develop > incrementally" section in the LLVM developer policy. At least the JSON parts > should be split off into a separate patch and tested independently. > - however, the choice of the JSON library is also an open question. We > currently have at least three options to choose from: > - llvm/Support/JSON.h > - lldb/Utility/JSON.h > - debugserver/source/JSON.h > > Of these, the third one is the one I'd least expect to be used here. Ok, I removed the debugserver reliance for the JSON parser and I have recoded it to use the LLVM JSON parser. Will post patch soon. > > > - Since this is essentially starting a new-subproject, I think it's in place > to discuss various conventions. E.g., right now, this seems to use a mixture > of UpperCamel and snake_case, and so isn't very consistent with neither llvm > nor lldb naming conventions. I will fix these things before checkin and run clang-format https://reviews.llvm.org/D50365 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
