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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to