On 23 September 2016 at 17:12, Sjoerd Meijer <sjoerd.mei...@arm.com> wrote: > I don't think I am committing at will: I thought it was okay to commit > directly because it is a simple fix (or should be) for my own previous > half-working patch, but I don't mind going through phab.
A review would have caught many issues with this patch, for example, the no default issue, and the fact that it has no tests at all. As a rule of thumb, wait until you get several no-comments-LGTM reviews in a row in the same area to start committing small patches before review in that area. Just a few changes isn't enough. > I am indeed suspecting it is missing default case (probably got confused > because e.g. ThreadModel also doesn't have one, at least not there). Please, also do a build on ARM, and at least try to run check-all, as that will catch most bugs. And don't forget to add tests. cheers, --renato _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
