-
Notifications
You must be signed in to change notification settings - Fork 31
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 WildFly 25 #210
add WildFly 25 #210
Conversation
…WildFly 25 and above legacy security was removed in WildFly 25
= ManagementClient.online(OnlineOptions.standalone().localDefault().build()).version(); | ||
Assume.assumeFalse("Legacy security was removed in WildFly 25.", | ||
serverVersion.greaterThanOrEqualTo(ServerVersion.VERSION_18_0_0)); | ||
} |
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.
Just a proposal - wouldn't be better to create some common function for this and use it in all relevant places instead of having the very same code on multiple places? 👼
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.
I was thinking about it but in the end, I followed the same convention as it is in elytron commands, there is a similar check in every elytron command.
@@ -43,6 +44,10 @@ private AddAuthorizationModule(Builder builder) { | |||
@Override | |||
public void apply(OnlineCommandContext ctx) throws CliException, CommandFailedException, IOException, | |||
TimeoutException, InterruptedException { | |||
if (ctx.version.greaterThanOrEqualTo(ServerVersion.VERSION_18_0_0)) { | |||
throw new AssertionError("Legacy security was removed in WildFly 25."); | |||
} |
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.
Again, having some common function would be probably nicer. But I don't insist.
Just two comments but LGTM otherwise :) |
intentionally adding only a single WF version per PR to leverage GH CI
depends on #206, #207, #208 and #209