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

Enable @ConstrainedTo on Features #4055

Merged
merged 3 commits into from
Feb 18, 2019

Conversation

jansupol
Copy link
Contributor

@jansupol jansupol commented Feb 9, 2019

Signed-off-by: Jan Supol jan.supol@oracle.com

Signed-off-by: Jan Supol <jan.supol@oracle.com>
@jansupol jansupol force-pushed the constrinedtoonfeatures branch from a9c6925 to c7b3881 Compare February 9, 2019 00:20
enabledFeatureClasses.add(registration.getFeatureClass());
enabledFeatures.add(feature);
final RuntimeType runtimeTypeConstraint = registration.getFeatureRuntimeType();
if (runtimeTypeConstraint == null || type.equals(runtimeTypeConstraint)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This check should be performed before line 674, so we do not create an injected instance of a Feature that should not exist in this type of runtime.

Copy link
Contributor

@tomas-langer tomas-langer left a comment

Choose a reason for hiding this comment

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

Check for runtime type should be performed sooner - see comment in file.

Signed-off-by: Jan Supol <jan.supol@oracle.com>
Signed-off-by: Jan Supol <jan.supol@oracle.com>
Copy link
Contributor

@tomas-langer tomas-langer left a comment

Choose a reason for hiding this comment

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

LGTM

@senivam senivam merged commit 61e6399 into eclipse-ee4j:master Feb 18, 2019
@jansupol jansupol added the 2.29 label Feb 18, 2019
@senivam senivam added this to the 2.29 milestone Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants