[GitHub] [logging-log4j2] vy commented on issue #335: Import of LogstashLayout as JsonTemplateLayout

[GitBox <gi...@apache.org>]

vy commented on issue #335: Import of LogstashLayout as JsonTemplateLayout
URL: https://github.com/apache/logging-log4j2/pull/335#issuecomment-579728012
 
 
   > > I chose `JsonTemplateLayout` as the new module name, which clearly communicates the intent. I am not totally convinced with the package name `o.a.l.log4j.jackson.json.template.layout` though. I have my doubts about whether we can remove Jackson dependency in the future or not. This ambition does look neither feasible, nor practical in the short-term. That said, from an API perspective, why shall an internal detail like `jackson` get exposed in the package name? I am considering to place it under `o.a.l.log4j.layout.json.template`. Any ideas?
   > 
   > No need to put jackson in early like that I think. The package was moved around for modularization in 3.x which makes me think we can still rename things.
   
   :+1: I will rename the package to `o.a.l.log4j.layout.json.template` once the dust settles down. I don't want to introduce such a big change right now and corrupt the conversation history.
   
   > > `LogstashLayout` has quite some configuration knobs which renders it inconvenient to explain it in `/src/site/asciidoc/manual/layouts.adoc`. I have skimmed through the source code with the hope of finding a similar documentation pattern, but failed to do so. Would somebody mind shedding some light onto the convention I should follow here, please?
   > 
   > You can use other formats if that makes it easier, though we may have to get creative.
   
   I am okay with AsciiDoc, my point was, shall I create a dedicated page (e.g., `json-template.adoc`) just for `JsonTemplateLayout`, given the rest of the `Layout`s are briefly summarized in `layouts.adoc`. Anyway... I will go toward this direction -- better to ask for excuse, rather than waiting permission. :wink: 
   
   > > I have some test utility classes that I want to leverage in performance tests, but test classpath of `log4j-jackson-json-template` module is not accessible by `log4j-perf` module. What is the work around here?
   > 
   > You can add the tests jar as a dependency to log4j-perf.
   
   Hrm... I didn't know that was possible. I will investigate how I can do that. If you happen to feel like helping, hints are welcome.
   
   > > I do not intend to implement `JsonLayout` by leveraging `JsonTemplateLayout`, at least, not now. I would rather just leave it be and get deprecated eventually. Put another way, is this PR the right place to address this issue?
   > 
   > Can likely be done as a follow up.
   
   :+1: 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services