vy commented on code in PR #3877:
URL: https://github.com/apache/logging-log4j2/pull/3877#discussion_r2284449980
##########
src/site/antora/modules/ROOT/pages/manual/dependencyinjection.adoc:
##########
Review Comment:
> `ConfigurableInstanceFactory` can create `@Configurable` namespace plugins
...
This is the very first occurrence of `namespace` keyword and it is used
without any definition and/or explanation. I think _names_ and _namespaces_ are
the two most confusing concepts of our DI system, which deserve at least a
dedicated paragraph on them. _Qualifiers_ and _scopes_ exponentially complicate
this nomenclature too.
##########
src/site/antora/modules/ROOT/pages/manual/dependencyinjection.adoc:
##########
Review Comment:
> as _bundles_ which are instances or injectable `Class<T>` instances that
contain one or more annotated factory methods.
_"Bundle"_ is a very convoluted term in the Java ecosystem. Above statement
literally describes a _"factory collection/set"_, which we can consider
replacing `bundle` with as a more self-explanatory term.
##########
src/site/antora/modules/ROOT/pages/manual/dependencyinjection.adoc:
##########
Review Comment:
> ... Lastly, no-arg methods annotated with `@Inject` are invoked which can
be useful for post-injection initialization logic. Additional strategies for
resolving factories for unbound keys may be registered which are consulted
first before falling back to this `@Inject` reflection logic.
Do we have a use case for every single logic described above? For instance,
I was not able to find a single usage of `@Inject` on a no-arg method. In a
follow-up PR, can we remove and all other functionalities that do not (yet)
serve a purpose?
##########
log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/Named.java:
##########
@@ -32,8 +29,8 @@
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER,
ElementType.TYPE_USE})
@Documented
-@NameProvider(NamedQualifierNameProvider.class)
-@AliasesProvider(NamedQualifierNameProvider.class)
+@NameProvider
+@AliasesProvider(offset = 1)
Review Comment:
I'm confused by `offset` and its function. (Yes, I've read its Javadoc.)
Would you mind explaining it a little bit more, please?
##########
src/site/antora/modules/ROOT/pages/manual/dependencyinjection.adoc:
##########
@@ -53,7 +53,7 @@ Injection points are injectable fields or parameters where a
dependency should b
_Injectable fields_ are fields annotated with `@Inject` or a qualifier
annotation.
_Injectable methods_ are methods annotated with `@Inject` or are not annotated
with a factory annotation and have at least one parameter annotated with a
qualifier annotation.
_Injectable constructors_ are constructors annotated with `@Inject`; only one
such constructor should exist per class.
-When a field or parameter is annotated with a name-providing annotation (i.e.,
an annotation annotated with
`@org.apache.logging.log4j.plugins.name.NameProvider`), then the provided name
or name of the field or parameter are included in the `Key<T>` for the
injection point.
+When a field or parameter is annotated with a name-providing annotation (i.e.,
an annotation annotated with `@org.apache.logging.log4j.plugins.NameProvider`),
then the provided name or name of the field or parameter are included in the
`Key<T>` for the injection point.
Review Comment:
```suggestion
When a field or parameter is annotated with `@NameProvider`, then the
provided name or name of the field or parameter are included in the `Key<T>`
for the injection point.
```
##########
log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/Namespace.java:
##########
@@ -23,7 +23,7 @@
import java.lang.annotation.Target;
/**
- * Annotations to separate {@link
org.apache.logging.log4j.plugins.name.NameProvider} names into namespaces.
+ * Annotations to separate {@link NameProvider} names into namespaces.
* For example, the {@linkplain Configurable Core namespace} is used with the
{@link Node} API, while the TypeConverter
* namespace is used with the {@link
org.apache.logging.log4j.plugins.convert.TypeConverter} API.
Review Comment:
I think this example needs to be explained more.
##########
log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/NameProvider.java:
##########
@@ -14,18 +14,20 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.logging.log4j.plugins.name;
+package org.apache.logging.log4j.plugins;
-import java.lang.annotation.Annotation;
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
+/**
+ * Marks another annotation as one providing a name for an object. The name is
obtained from
+ * the annotation element named {@code value}. This element can be a {@code
String} or {@code String[]}.
+ * When specified as an array, the first element is used.
+ */
Review Comment:
> Marks another annotation as one providing a name for an object.
Oookay... `PluginElement`, `PluginValue`, etc. are `NameProvider`s. Got it.
> The name is obtained from the annotation element named `value`.
> This element can be a `String` or `String[]`.
> When specified as an array, the first element is used.
I am totally lost here. Would you mind elaborating on this.
##########
log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/Namespace.java:
##########
@@ -23,7 +23,7 @@
import java.lang.annotation.Target;
/**
- * Annotations to separate {@link
org.apache.logging.log4j.plugins.name.NameProvider} names into namespaces.
+ * Annotations to separate {@link NameProvider} names into namespaces.
Review Comment:
What does _"`NameProvider` names"_ mean? _"Names provided by
`NameProvider`s"_?
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
