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

Relax the scope requirement for public fields #496

Merged
merged 1 commit into from
Nov 8, 2023

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Nov 7, 2023

This is a TCK counterpart to the specification change that allows all pseudo-scoped beans to have public fields. It updates the text in tck-audit-cdi.xml files, fixes naming in the relevant tests and adds a test that a @Singleton bean can have a public field.

@Ladicek Ladicek added this to the CDI 4.1 milestone Nov 7, 2023
@Ladicek Ladicek requested a review from manovotn November 7, 2023 16:03
@Ladicek Ladicek force-pushed the public-fields-scope-requirement branch from 61bdd60 to 71942ea Compare November 7, 2023 16:04
@Ladicek Ladicek changed the title Relax the scope requirement on pseudo-scoped beans Relax the scope requirement for public fields Nov 7, 2023
@manovotn
Copy link
Contributor

manovotn commented Nov 7, 2023

@Ladicek shouldn't we add a test that verifies this for @Singleton?

@Ladicek
Copy link
Contributor Author

Ladicek commented Nov 8, 2023

We could, but it's not like we can ever be exhaustive. For normal scopes, we only test @RequestScoped, even though we could also test @ApplicationScoped easily 🤷

@manovotn
Copy link
Contributor

manovotn commented Nov 8, 2023

We could, but it's not like we can ever be exhaustive. For normal scopes, we only test @RequestScoped, even though we could also test @ApplicationScoped easily 🤷

True, I only mentioned this as it is a notable difference from previous version of specification.
I'll leave it up to you

@Ladicek
Copy link
Contributor Author

Ladicek commented Nov 8, 2023

I could add a test for @Singleton as well, but @Singleton is not a bean defining annotation, so I would have to create a stereotype, and I'm just lazy...

OK, I will do it :-)

This is a TCK counterpart to the specification change that allows all
pseudo-scoped beans to have public fields. It updates the text
in `tck-audit-cdi.xml` files, fixes naming in the relevant tests
and adds a test that a `@Singleton` bean can have a public field.
@Ladicek Ladicek force-pushed the public-fields-scope-requirement branch from 71942ea to b52a23d Compare November 8, 2023 10:50
@Ladicek
Copy link
Contributor Author

Ladicek commented Nov 8, 2023

Done.

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.

Thank you and sorry for the bother :)

@Ladicek Ladicek merged commit 986e0d0 into jakartaee:master Nov 8, 2023
@Ladicek Ladicek deleted the public-fields-scope-requirement branch November 8, 2023 11:32
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.

2 participants