-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove App{Archive,ArchiveConfig,Deployment}, rename AppArchiveBuilder and ContextBuilder #506
remove App{Archive,ArchiveConfig,Deployment}, rename AppArchiveBuilder and ContextBuilder #506
Conversation
This is just a first round of improvements I have in mind. More will follow for sure. Since @graemerocher started improving the language model part, I started looking at the actual extension API, so that we don't clash (hopefully :-) ). |
/** | ||
* Registers custom context as configured by the returned {@link ContextConfig}. | ||
*/ | ||
ContextConfig addContext(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point, maybe we should just have addContext(Class<? extends AlterableContext>)
(which is the only mandatory part) and leave ContextConfig
purely for the optional stuff?
api/src/main/java/jakarta/enterprise/inject/build/compatible/spi/ScannedClasses.java
Outdated
Show resolved
Hide resolved
88f492d
to
7197328
Compare
…r and ContextBuilder The `AppArchive`, `AppArchiveConfig` and `AppDeployment` types were redundant. Moreover, they looked like they can access whole world, which prevents certain kinds of implementations. (Some effort was made to design the API in these types so that whole world access is not necessary, but it's a leaky abstraction.) For those reasons, these types are removed altogether. The `AppArchiveBuilder` type is renamed to `ScannedClasses`, which is more descriptive (and shorter). The `addSubtypesOf` method is removed, because it can't be implemented in certain environments (notably, Portable Extensions). The `ContextBuilder` type is renamed to `ContextConfig`, because it isn't really used in the usual builder way and is quite close to the other existing `*Config` types. Additionally, some documentation is improved, especially for types in the `@Discovery` phase.
7197328
to
deb6d74
Compare
Great this makes life much simpler. You working the AnnotationBuilder PR now? |
I do, yes. Should be up shortly -- I have to resist the urge to change too many things at once :-) |
The
AppArchive
,AppArchiveConfig
andAppDeployment
typeswere redundant. Moreover, they looked like they can access whole
world, which prevents certain kinds of implementations. (Some
effort was made to design the API in these types so that whole
world access is not necessary, but it's a leaky abstraction.)
For those reasons, these types are removed altogether.
The
AppArchiveBuilder
type is renamed toScannedClasses
,which is more descriptive (and shorter). The
addSubtypesOf
method is removed, because it can't be implemented in certain
environments (notably, Portable Extensions).
The
ContextBuilder
type is renamed toContextConfig
, becauseit isn't really used in the usual builder way and is quite close
to the other existing
*Config
types.Additionally, some documentation is improved, especially for
types in the
@Discovery
phase.