Skip to content
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

add BeanConfigurator.priority #532

Merged
merged 1 commit into from Sep 22, 2021
Merged

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Sep 22, 2021

This is to allow setting a priority for synthetic beans created
using BeanConfigurator.

I briefly considered if priority should be added to
BeanAttributesConfigurator as well, but that would have
a ripple effect. It also doesn't reflect the present reality.
A custom Bean implementation can be added a priority
by implementing the Prioritized interface. Adding only
BeanConfigurator.priority is a perfect mirror of that
in the configurator API.

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 22, 2021

This alters the Portable Extensions API, so I'm not adding Graeme for review. The only relationship to Lite is that this addition will make it possible to completely implement the Build Compatible Extensions API on top of Portable Extensions.

Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to allow setting a priority for synthetic beans created
using `BeanConfigurator`.

I briefly considered if `priority` should be added to
`BeanAttributesConfigurator` as well, but that would have
a ripple effect. It also doesn't reflect the present reality.
A custom `Bean` implementation can be added a priority
by implementing the `Prioritized` interface. Adding only
`BeanConfigurator.priority` is a perfect mirror of that
in the configurator API.
@Ladicek Ladicek merged commit 7c21841 into jakartaee:master Sep 22, 2021
@Ladicek Ladicek deleted the synth-bean-priority branch September 22, 2021 11:18
@manovotn
Copy link
Contributor

@Ladicek Btw we should create a tracking issue for TCK test or send one right away (we could probably use existing weld test) so that we don't forget.

@starksm64
Copy link
Contributor

I added a TCK issue

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 23, 2021

Whoops! Yes, good point. @manovotn can you please point me to the existing Weld test? I'll try submitting a PR to the TCK.

@manovotn
Copy link
Contributor

@Ladicek this one - https://github.com/weld/core/tree/master/tests-arquillian/src/test/java/org/jboss/weld/tests/alternatives/customBeanPriority

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants