Hi, Thanks for extracting some of the classes. I think you could have gone further, with the Filters as they are service implementation and being registered inside the activate method does a good job of hiding them from someone scanning the code base. I don't think it's important enough to do anything about unless you think it matters. Best Regards Ian
On 22 May 2015 at 11:18, Chetan Mehrotra <chetan.mehro...@gmail.com> wrote: > Thanks for all the feedback. I have opened SLING-4739 and committed > the code to svn. The documentation is also moved to Sling site [2] > > > For me a Sling- prefix > > is enough to make it clear that it's a private, Sling-specific, > > header, but I don't have a strong opinion about it. > > @Robert - Code now uses Sling-Tracers and Sling-Tracer-Config has header > name > > > Is there a fundamental reason the LogTracer class must put ServletFilters > > as inner classes ? It has 8 inner classes which generally makes > > understanding the code harder and leads to the temptation to reference > > internal implementation details across class boundaries. I understand > other > > languages have that flexibility by mistake or intention, but the pattern > > when used in Java tends to lead to problems downstream. > > @Ian - I prefer use of inner class if the class is not to be exposed > or not to be known to outer world. However I agree in this case it > went bit overboard, so committed code is refactored to simplify the > logic. Hopefully this is simpler! > > Chetan Mehrotra > [1] https://issues.apache.org/jira/browse/SLING-4739 > [2] http://sling.apache.org/documentation/bundles/log-tracers.html > > > On Thu, May 21, 2015 at 2:11 PM, Robert Munteanu <romb...@apache.org> > wrote: > > On Thu, May 21, 2015 at 11:38 AM, Chetan Mehrotra > > <chetan.mehro...@gmail.com> wrote: > >> On Thu, May 21, 2015 at 2:00 PM, Robert Munteanu <romb...@apache.org> > wrote: > >>> AEM -> Sling is self-explanatory, and the X- prefix is deprecated with > >>> RFC6648 [1] > >> > >> Well not sure. That recommendation is meant for headers which are > >> meant to be used in a wider context. Header which are very much > >> specific to application are better prefixed to indicate that they are > >> custom ones IMHO [2]. So I would prefer naming them X-Sling-Tracers. > > > > Right, it doesn't apply to 'private' headers. For me a Sling- prefix > > is enough to make it clear that it's a private, Sling-specific, > > header, but I don't have a strong opinion about it. > > > > Robert >
