Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-06 Thread Erick Erickson
> On March 3, 2018, 4:41 a.m., Tomás Fernández Löbbe wrote: > > LGTM. I think you need to add org.apache.logging.log4j.** to forbidden APIs > > Erick Erickson wrote: > It turns out not. Thanks to Shawn I now know the shim is used, which > means we can (and do) still use log4j calls in the c

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-06 Thread Tomás Fernández Löbbe
> On March 3, 2018, 4:41 a.m., Tomás Fernández Löbbe wrote: > > LGTM. I think you need to add org.apache.logging.log4j.** to forbidden APIs > > Erick Erickson wrote: > It turns out not. Thanks to Shawn I now know the shim is used, which > means we can (and do) still use log4j calls in the c

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-06 Thread Shawn Heisey
> On March 3, 2018, 4:41 a.m., Tomás Fernández Löbbe wrote: > > LGTM. I think you need to add org.apache.logging.log4j.** to forbidden APIs > > Erick Erickson wrote: > It turns out not. Thanks to Shawn I now know the shim is used, which > means we can (and do) still use log4j calls in the c

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-06 Thread Tomás Fernández Löbbe
> On March 3, 2018, 4:41 a.m., Tomás Fernández Löbbe wrote: > > LGTM. I think you need to add org.apache.logging.log4j.** to forbidden APIs > > Erick Erickson wrote: > It turns out not. Thanks to Shawn I now know the shim is used, which > means we can (and do) still use log4j calls in the c

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-06 Thread Shawn Heisey
> On March 3, 2018, 4:41 a.m., Tomás Fernández Löbbe wrote: > > LGTM. I think you need to add org.apache.logging.log4j.** to forbidden APIs > > Erick Erickson wrote: > It turns out not. Thanks to Shawn I now know the shim is used, which > means we can (and do) still use log4j calls in the c

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-05 Thread Tomás Fernández Löbbe
> On March 3, 2018, 4:41 a.m., Tomás Fernández Löbbe wrote: > > LGTM. I think you need to add org.apache.logging.log4j.** to forbidden APIs > > Erick Erickson wrote: > It turns out not. Thanks to Shawn I now know the shim is used, which > means we can (and do) still use log4j calls in the c

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-04 Thread Erick Erickson
> On March 4, 2018, 1:28 a.m., Erick Erickson wrote: > > there's still a reference to log4j.properties in solr/bin/solr.cmd changed > > in the patch I'll upload shortly. > > > > NOTE: I've changed everything _except_ the references to log4j 1.2 in > > solr/sever/ivy.xml and solr/core/ivy.xml i

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-04 Thread Erick Erickson
> On March 4, 2018, 2:04 a.m., Erick Erickson wrote: > > A couple more things. > > > > WDYT about including > > > > in the Root? > > > > > > And let's just copy the log4j2.xml file from server/resources to > > example/resources. > > > > example/resources/log4j2.xml has this in the patt

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-04 Thread Erick Erickson
> On March 3, 2018, 4:41 a.m., Tomás Fernández Löbbe wrote: > > LGTM. I think you need to add org.apache.logging.log4j.** to forbidden APIs It turns out not. Thanks to Shawn I now know the shim is used, which means we can (and do) still use log4j calls in the code. > On March 3, 2018, 4:41 a.

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-04 Thread Erick Erickson
> On March 4, 2018, 1:28 a.m., Erick Erickson wrote: > > solr/core/ivy.xml > > Lines 56 (patched) > > > > > > Should this still be 1.2? > > Shawn Heisey wrote: > This is the compatibility shim. It is part of log

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-03 Thread Erick Erickson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65888/#review198588 --- A couple more things. WDYT about including in the Root? A

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-03 Thread Erick Erickson
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65888/#review198587 --- there's still a reference to log4j.properties in solr/bin/solr.cmd

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-02 Thread Tomás Fernández Löbbe
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65888/#review198580 --- Ship it! LGTM. I think you need to add org.apache.logging.log4j

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-02 Thread Varun Thacker
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65888/ --- (Updated March 3, 2018, 2:51 a.m.) Review request for lucene. Repository: luc

Re: Review Request 65888: Upgrade Solr to use Log4J2

2018-03-02 Thread Varun Thacker
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65888/ --- (Updated March 2, 2018, 9:45 p.m.) Review request for lucene. Summary (update