> On May 27, 2022, at 6:54 AM, Volkan Yazıcı <[email protected]> wrote:
>
> Matt, some more questions/remarks:
>
> 1. AFAIU, plugin `key` is the normalized `name`. Right? If so, what is
> the purpose of the `name`?
See PluginElementVisitor (at least in 2.x)
@Override
public PluginEntry visitType(final TypeElement e, final Plugin plugin) {
Objects.requireNonNull(plugin, "Plugin annotation is null.");
final PluginEntry entry = new PluginEntry();
entry.setKey(plugin.name().toLowerCase(Locale.US));
entry.setClassName(elements.getBinaryName(e).toString());
entry.setName(Plugin.EMPTY.equals(plugin.elementType()) ? plugin.name() :
plugin.elementType());
entry.setPrintable(plugin.printObject());
entry.setDefer(plugin.deferChildren());
entry.setCategory(plugin.category());
return entry;
}
Some elements (such as KeyValuePair) don’t specify an elementType. In that
case the name attribute is set to the plugin’s name attribute value. Otherwise
it is the elementType. The key is always the Plugin’s name attribute.
> 2. I was expecting `PluginEntry` to be simplified similar to `Plugin`
> and only contain a `String key`, `String name`, and `String className`.
Why would you expect that? All the fields are used by the Plugin system and
need to be in the plugin metadata to load it. Remember, at this point we don’t
use the annotations any more.
> For
> `@Configurable`, we could create a `ConfigurablePlugin` class extending
> from `Plugin` with extra `elementType`, `printable`, etc. fields. Generated
> `Log4jPlugins.java` could instantiate either `Plugin` or
> `ConfigurablePlugin` depending on the need. In its current state, `Plugin`
> looks pretty generic, though the rest still resembles the past architecture.
IMO what you propose just makes things unnecessarily complicated.
> 3. What is the purpose of populating interfaces (i.e.,
> `PluginEntry#interfaces`) at compile-time? What does use them and how?
I’m interested in this too as PluginProcessor doesn’t seem to be setting any
values.
> 4. I see quite some overlap between `plugins.condition` and
> `plugins.validation`. I would prefer the latter to be modernized and merged
> into the former.
> 5. I would prefer `Configurable`, `Node`, et al. to be moved to their
> own `plugins.config` package.
Not sure I agree with this.
> 6. I would prefer `di.Scope` et al. to be moved to their own
> `plugins.scope` package – or at least, be located in the same package. For
> instance, consider `plugins.di.Scope` vs. `plugins.ScopeType`.
I agree it should be moved. It could be either plugins.scope or
plugins.di.scope.
I’d prefer the former since it is related to the Plugin annotation
Ralph
