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

Reply via email to