Sorry for the missing links. Here they are: [1] https://github.com/vy/log4j2/blob/json-template-layout/log4j-layout-json-template/src/main/java/org/apache/logging/log4j/layout/json/template/util/JsonReader.java [2] https://github.com/vy/log4j2/blob/json-template-layout/log4j-layout-json-template/src/main/java/org/apache/logging/log4j/layout/json/template/util/JsonWriter.java [3] https://github.com/vy/log4j2/blob/json-template-layout/log4j-layout-json-template/src/test/java/org/apache/logging/log4j/layout/json/template/util/JsonReaderTest.java [4] https://github.com/vy/log4j2/blob/json-template-layout/log4j-layout-json-template/src/test/java/org/apache/logging/log4j/layout/json/template/util/JsonWriterTest.java [5] https://github.com/vy/log4j2/blob/json-template-layout/log4j-layout-json-template/src/main/java/org/apache/logging/log4j/layout/json/template/util/RecyclerFactories.java https://github.com/vy/log4j2/blob/json-template-layout/src/site/asciidoc/manual/json-template-layout.adoc#recycling-strategy [6] https://github.com/JCTools/JCTools/issues/289
On Wed, Mar 11, 2020 at 4:19 PM Volkan Yazıcı <[email protected]> wrote: > > My comments are inline. > > On Wed, Mar 11, 2020 at 4:01 PM Ralph Goers <[email protected]> > wrote: > > 1. Did you lose functionality by removing the dependencies? > > Except pretty printing, no. > > > 2. Did you have to add things like JSON parsing to > > remove the dependencies? > > Yes, I have implemented simple JsonReader[1] and JsonWriter[2] > classes. Both are well tested[3][4]. > > > 3. Under what circumstances is JCTools optional? > > User can pick different allocation strategies: dummy, thread-local, > and queue. "queue" also accepts an optional "java.util.Queue(int > capacity)" factory method parameter. There, by default, if JCTools is > in the classpath, we use MpmcArrayQueue; otherwise fallback to > ArrayBlockingQueue. (The user can also explicitly demand and enforce > the queue.) The logic is available here[5]. > > > 4. Does JCTools provide a module-info.class or > > Automatic-Module-Name header in MANIFEST.MF? > > As you know[6] better than I do, no. > > > The goal should not be to move things to core. The goal should > > be to make them simple for users to integrate. > > I will interpret this as "keep it in the module". > > > As for benchmarks, if you have current benchmarks > > then include them. They don’t have to be updated with > > each release but the page should indicate what release > > they were for. > > Okay. > > > Speaking of documentation, if you are creating a separate > > page or pages for JsonTemplateLayout make sure there is > > an entry in the general Layout page that has a general > > description and a link to the full documentation. > > Yes, I have implemented it as you described.
