DavidSpickett wrote: I have reverted this PR as it should not have been merged as it was. I'll give you some benefit of the doubt since there was an approval but from context, I think it was pretty clear that it was for the settings part specifically.
Some of my comments were resolved but as far as I can tell, you either did not address them or you did and the text ended up in the wrong places. For example the error message I suggested has ended up in the release notes. I also had questions about the use case for this feature and the terminology of security vs. safety. You may not think these are important, and perhaps they are not, but we treat every contribution seriously so please engage with feedback sincerely. Today it might be a pedantic wording question, tomorrow it might be a bug that would have cost users weeks of their time. If I did not explain that specific comment well enough, given that in a lot of cases the two words mean mostly the same thing, please say so and I will do my job as a reviewer and communicate more clearly. In general while I am impressed with the amount of work you are doing, at least from here, I think you are going too fast. Finding your limits is fine, we all have to push boundaries every so often, but take a step back and maybe sit on your changes for a bit longer before doing the next step. Often you'll come back with fresh eyes and realise you could do better. If you still want to pursue these changes, please open a new PR and we can discuss them there. FWIW I don't expect to have major problems with this technically, but we must have that discussion. https://github.com/llvm/llvm-project/pull/183237 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
