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

WFLY-12006 EJB3 IIOP attributes enable-by-default and use-qualified-name should be required #12237

Merged
merged 2 commits into from Apr 24, 2019

Conversation

@soul2zimate
Copy link
Contributor

soul2zimate commented Apr 18, 2019

https://issues.jboss.org/browse/WFLY-12006

Update attributes enable-by-default and use-qualified-name to required.
Because enable-by-default and use-qualified-name must be required during parsing at https://github.com/wildfly/wildfly/blob/16.0.0.Final/ejb3/src/main/java/org/jboss/as/ejb3/subsystem/EJB3Subsystem12Parser.java#L227

@wildfly-ci wildfly-ci added the deps-ok label Apr 18, 2019
Copy link
Contributor

bstansberry left a comment

EJB3IIOPAdd can be cleaned up a bit too; in performBoottime when it reads these values it should assign to 'boolean' variables, not 'Boolean'.

Technically the EJB subsystem management API version should be bumped for this, but, meh. I think it was never possible to effectively not define these if the resource was added.

@@ -57,12 +57,14 @@
new SimpleAttributeDefinitionBuilder(EJB3SubsystemModel.USE_QUALIFIED_NAME, ModelType.BOOLEAN, true)
.setAllowExpression(true)
.setFlags(AttributeAccess.Flag.RESTART_NONE)
.setRequired(true)

This comment has been minimized.

Copy link
@bstansberry

bstansberry Apr 22, 2019

Contributor

Please remove the 'true' from L57 as that says the opposite of this setRequired(true).

.build();

static final SimpleAttributeDefinition ENABLE_BY_DEFAULT =
new SimpleAttributeDefinitionBuilder(EJB3SubsystemModel.ENABLE_BY_DEFAULT, ModelType.BOOLEAN, true)
.setAllowExpression(true)
.setFlags(AttributeAccess.Flag.RESTART_NONE)
.setRequired(true)

This comment has been minimized.

Copy link
@bstansberry

bstansberry Apr 22, 2019

Contributor

Please remove the 'true' from L64 as that says the opposite of this setRequired(true).

soul2zimate added 2 commits Apr 18, 2019
…ame should be required
@soul2zimate soul2zimate force-pushed the soul2zimate:WFLY-12006-master branch from 0efc900 to bac1a1f Apr 23, 2019
@soul2zimate

This comment has been minimized.

Copy link
Contributor Author

soul2zimate commented Apr 23, 2019

Thanks, I have updated as per review request. I also removed opposite optional value "true" in three other classes.

@bstansberry bstansberry merged commit 216a681 into wildfly:master Apr 24, 2019
7 checks passed
7 checks passed
Dependency Tree (Pull Request) - merge TeamCity build finished
Details
Linux - JDK 11 (Pull Request) - merge TeamCity build finished
Details
Linux - JDK 8 Finished TeamCity Build WildFly / Pull Request / Linux - JDK 8 : Tests passed: 4857, ignored: 134
Details
Linux - elytron - JDK 8 (Pull Request) - merge TeamCity build finished
Details
Linux with security manager - JDK 8 Finished TeamCity Build WildFly / Pull Request / Linux SM - JDK 8 : Tests passed: 4513, ignored: 152
Details
Windows - JDK 11 Finished TeamCity Build WildFly / Pull Request / Windows - JDK 11 : Tests passed: 4848, ignored: 141
Details
Windows - JDK 8 Finished TeamCity Build WildFly / Pull Request / Windows - JDK 8 : Tests passed: 4850, ignored: 139
Details
@soul2zimate soul2zimate deleted the soul2zimate:WFLY-12006-master branch Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.