Log version and capabilities when starting up #457
Conversation
|
Oh and the output looks like: I've also changed the level of |
|
Do we want to log the extensions or the capabilities? I might have misunderstood but I thought that there wasn't a 1-1 mapping for them. |
| @@ -683,6 +686,9 @@ private void generateFieldInjection(String processorClassName, MethodCreator bui | |||
| } else if (field.injectionType == InjectionType.LIST) { | |||
| ResultHandle val = buildStepMc.invokeVirtualMethod(ofMethod(BuildContext.class, "consumeMulti", List.class, Class.class), buildStepMc.getMethodParam(0), buildStepMc.loadClass(field.consumedTypeName)); | |||
| buildStepMc.writeInstanceField(FieldDescriptor.of(processorClassName, field.element.getSimpleName().toString(), List.class), p, val); | |||
| } else if (field.injectionType == InjectionType.OPTIONAL) { | |||
| ResultHandle val = buildStepMc.invokeVirtualMethod(ofMethod(BuildContext.class, "consumeOptional", Optional.class, Class.class), buildStepMc.getMethodParam(0), buildStepMc.loadClass(field.consumedTypeName)); | |||
There was a problem hiding this comment.
It would be better to just call consume and wrap the result in Optional.ofNullable instead. This should be done in conjunction with the (existing) OPTIONAL flag in org.jboss.builder.ConsumeFlag.
There was a problem hiding this comment.
Hm, what is org.jboss.builder.ConsumeFlag do? How is it used?
There was a problem hiding this comment.
You specify it on the consumes method of the build chain builder, when the generated code is declaring the items that it consumes. Note that the new reflection-based processor already supports optional items in this way.
There was a problem hiding this comment.
Is there a branch with this new reflection-based processor? I'm still lost..
There was a problem hiding this comment.
I think I have it:
https://github.com/jbossas/protean-shamrock/blob/master/core/deployment-api/src/main/java/org/jboss/shamrock/annotations/BuildAnnotationProcessor.java#L618
This is for field injection, right?
There was a problem hiding this comment.
There was a problem hiding this comment.
It's obviously not ready yet but getting closer every day.
There was a problem hiding this comment.
Yeah that's the right one, and you'll see that the other handlers are right below it.
| /** | ||
| * An optional build item that may be consumed as an {@code Optional}. | ||
| */ | ||
| public abstract class OptionalBuildItem extends SimpleBuildItem { |
There was a problem hiding this comment.
Ok, so should I remove this completely and make use of the ConsumeFlag somehow? This builder is getting quite complex and it's hard to know where to start...
There was a problem hiding this comment.
Yes, see the above comment for info.
|
@gsmet Well, from user POV we should probably log capabilities because users don't care about extensions. And you're right extension != capability. |
|
@dmlloyd Oh, I didn't know about |
mkouba
force-pushed
the
issue-283-versionandcapabilities
branch
2 times, most recently
from
492acaf
to
98156d8
Compare
|
@dmlloyd Second attempt, pls review ;-) |
|
TBH I don't like the approach of having to declare every extension as a capability. I wonder if there's some way to do it automatically? |
|
Hm, I think that an extension may bring in zero or more capabilities. |
|
I think we should be listing extensions not capabilities. Capabilities were intended to be internal, and could represent some internal feature that is not particularly useful to users. |
|
@stuartwdouglas @dmlloyd Ok. So if capabilities are designed to be internal we should introduce a new build item, maybe "feature" and list the features? Re automation - extensions don't have any descriptor yet and as I said I think that an extension may register zero or more features (from user POV). |
mkouba
force-pushed
the
issue-283-versionandcapabilities
branch
from
98156d8
to
51aa65b
Compare
- also make it possible to inject optional build item - improve ServiceStartBuildItem javadoc - resolves #283
mkouba
force-pushed
the
issue-283-versionandcapabilities
branch
from
51aa65b
to
225b58f
Compare
|
Introduced |
|
If there are no objections I'd like to merge this today. We can revisit the automatic registration later. |
Also adds optional build item - @dmlloyd I know that the impl is probably not good but I needed something that works (or at least seem to work). Feel free to replace the code with something more robust.