r282255 - Fix for r280064 that added options for fp denormals and exceptions.
Renato Golin via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 23 10:20:39 PDT 2016
On 23 September 2016 at 17:12, Sjoerd Meijer <Sjoerd.Meijer at 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
More information about the cfe-commits
mailing list