On 05/10/18 19:46, Christopher Schultz wrote:
> Mark,
>
> On 10/5/18 06:22, Mark Thomas wrote:
>> On 05/10/18 10:42, Rémy Maucherat wrote:
>>> On Fri, Oct 5, 2018 at 11:40 AM Mark Thomas <ma...@apache.org>
>>> wrote:
>>>
>>>> On 04/10/18 22:07, isa...@apache.org wrote:
>>>>> Author: isapir Date: Thu Oct 4 21:07:54 2018 New Revision:
>>>>> 1842849
>>>>>
>>>>> URL: http://svn.apache.org/viewvc?rev=1842849&view=rev Log:
>>>>> System.load() expects absolute path.
>>>>
>>>> Remember to consider whether or not any changes you make to
>>>> trunk should be back-ported to 8.5.x and 7.0.x. Generally,
>>>> changes are back-ported unless they require changing a public
>>>> API (as defined in RELEASE-NOTES) or are considering likely to
>>>> cause a regression.
>>>>
>>>> <snip/>
>>>>
>>>>> Modified:
>>>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf
> .java
>>>>>
>>>>
> URL:
>>>> http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/uti
> l/net/openssl/TestOpenSSLConf.java?rev=1842849&r1=1842848&r2=1842849&vie
> w=diff
>>>>>
>>>>
>>>>
> ========================================================================
> ======
>>>>> ---
>>>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf
> .java
>>>>
>>>>
> (original)
>>>>> +++
>>>> tomcat/trunk/test/org/apache/tomcat/util/net/openssl/TestOpenSSLConf
> .java
>>>>
>>>>
> Thu Oct 4 21:07:54 2018
>>>>> @@ -87,7 +87,11 @@ public class TestOpenSSLConf extends Tom
>>>>>
>>>>> @Test public void testOpenSSLConfCmdCipher() throws Exception
>>>>> { - log.info("Found OpenSSL version 0x" +
>>>> Integer.toHexString(OPENSSL_VERSION));
>>>>> + if (TesterSupport.isOpensslAvailable()) +
>>>>> log.info("Found OpenSSL version 0x" +
>>>> Integer.toHexString(OPENSSL_VERSION));
>>>>> + else + log.warn("OpenSSL not found: " +
>>>> TesterSupport.OPENSSL_ERROR);
>>>>> +
>>>>
>>>> The Tomcat style is to always use { and } even for one line for
>>>> clarity.
>>>>
>>>> Due to the age of the code base, there are a mix of styles.
>>>> Generally, we try and move code towards the currently accepted
>>>> style as we change it.
>>>>
>>>
>>> +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.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
