On 09/10/18 07:04, Igal Sapir wrote: > Mark, > > On Mon, Oct 8, 2018 at 2:52 AM Mark Thomas <ma...@apache.org> wrote: > >> On 05/10/18 19:46, Christopher Schultz wrote: >> <snip/> >>>>> +1 a lack of { } is too big a possible bug source to ignore. >>> >>>> I just tried enabling the CheckStyle test for this. There were >>>> just under three thousand errors. >>> >>>> I'm wondering if it is worth going through the code base fixing >>>> these. >>> >>> I'm nearly -1 on this, mostly because it will make back-porting stuff >>> a total PITA. >> >> Fair enough. I don't need much convincing not to do it as I have plenty >> of other stuff on my TODO list. >> >>> Definitely opportunistically "upgrade" code we find here and there, >>> but I don't think it's worth taking a day or two to add missing >>> explicit blocks everywhere. >> >> ACK. >> >>>> On a related topic, I did notice several instance of the >>>> following: >>> >>>> if (a == b) ... if (a == c) ... if (a == d) ... >>> >>>> that could be more efficiently written as: >>> >>>> if (a == b) { ... } else if (a == c) { ... } else if (a == d) { >>>> ... } >>> >>> That would be nice. Sounds like a BZ issue that could have a >>> "beginner" keyword attached. >> >> Good idea. Feel free to add that if I don't get there first. >> >> > Did you notice the consecutive if statements by chance, or does CheckStyle > report those?
I noticed them by chance while I was looking at CheckStyle warnings for if statements not using { ... } > I just imported the CheckStyle profile into IntelliJ IDEA > and I see more than 82,000 warnings, many of which complaining of missing > Javadoc comments and lines longer than 80 characters. If you import the checkstyle configs from res/checkstyle.xml and apply them as per https://github.com/apache/tomcat/blob/trunk/build.xml#L565 you should get a clean build. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org