Update the configuration docs in the extension guide #736
Conversation
| @@ -159,95 +159,140 @@ file will be indexed. | |||
|
|
|||
| == Configuration | |||
|
|
|||
| Simple configuration is done via the https://github.com/eclipse/microprofile-config[MicroProfile Config] `@org.eclipse.microprofile.config.inject.ConfigProperty` annotations. | |||
| Configuration in Shamrock is based on the SmallRye Config, an implementation of the MicroProfile Config specification. | |||
There was a problem hiding this comment.
Either on SmallRye Config or on the SmallRye Config project/component/library/whatever.
| } | ||
| ---- | ||
| <1> The FileConfig object is annotated with @ConfigGroup to indcate that this is an aggregate | ||
| <1> The `FileConfig` object is annotated with `@ConfigGroup` to indicate that this is an aggregate |
There was a problem hiding this comment.
You changed the name of FileConfig to File in the snippet above.
| configuration object containing a collection of configurable properties. | ||
| <2> Here the LoggingProcessor injects a FileConfig instance using the MicroProfile Config annotation with the property name "shamrock.log.file". | ||
| <2> The `@ConfigRoot` annotation indicates that this object is a configuration root group, whose property names will have a parent only of `shamrock.`. | ||
| <3> Here the `LoggingProcessor` injects a `FileConfig` instance automatically by detecting the `@ConfigRoot` annotation. |
| /** | ||
| * Logging configuration. | ||
| */ | ||
| @ConfigRoot(phase = ConfigPhase.STATIC_INIT) <2> |
There was a problem hiding this comment.
I don't see any explanation about the phases?
There was a problem hiding this comment.
Yeah I thought I'd leave this off for now until we fix the way roots are resolved (see email). It needs a little more discussion.
| @ConfigProperty(name = "shamrock.log.file") | ||
| FileConfig file; | ||
| <3> | ||
| Log log; |
There was a problem hiding this comment.
You don't even have to mark it with an @Inject annotation or something? I think I would have preferred having an injection marker. At least, you know that it's being injected.
There was a problem hiding this comment.
It's always injected because you can't have any non-injected fields on a configuration group class. Since you'd always need it, it would probably be a bit annoying to always specify @Inject.
| configuration object containing a collection of configurable properties. | ||
| <2> Here the LoggingProcessor injects a FileConfig instance using the MicroProfile Config annotation with the property name "shamrock.log.file". | ||
| <2> The `@ConfigRoot` annotation indicates that this object is a configuration root group, whose property names will have a parent only of `shamrock.`. | ||
| <3> Here the `LoggingProcessor` injects a `FileConfig` instance automatically by detecting the `@ConfigRoot` annotation. |
There was a problem hiding this comment.
This sentence is a bit confusing because at this point, you inject a Log not a File. And the @ConfigRoot annotation is not there but on the config object.
There was a problem hiding this comment.
Yep I forgot to update this after I went back and made some revisions. Will fix tomorrow.
| @ConfigGroup <1> | ||
| public class FileConfig { | ||
| public class File { |
There was a problem hiding this comment.
I know why you did it but tbh, I find this change pretty confusing and it was really clearer to have the object called SomethingConfig.
Take for example Hibernate Validator: I will have a HibernateValidator config object if I want hibernate-validator in my config path which looks pretty confusing.
It seems that there's no way to override the name in the @ConfigGroup annotation? Same for @ConfigRoot. Is it on purpose?
There was a problem hiding this comment.
Wondering if maybe you could just remove the Config suffix automatically (if the class is not called Config) when generating the property name?
There was a problem hiding this comment.
Yeah definitely: see #705 (comment)
|
Updated; please re-review. |
This would happen when there was nothing to be done but we still returned with the exit code instructing the startup script to execute jbang's output.
No description provided.