Skip to content

[KARAF-6797] More detailed description of the JAAS command options fo…#1142

Merged
jbonofre merged 1 commit intoapache:masterfrom
AndreVirtimo:master
Aug 10, 2020
Merged

[KARAF-6797] More detailed description of the JAAS command options fo…#1142
jbonofre merged 1 commit intoapache:masterfrom
AndreVirtimo:master

Conversation

@AndreVirtimo
Copy link
Contributor

The option "--realm" of the command jaas:realm-manage have to be combine with the option "--module" and vice versa. This wasn't mentioned in the help message of the command.

@jbonofre jbonofre self-requested a review August 10, 2020 05:29
public class ManageRealmCommand extends JaasCommandSupport {

@Option(name = "--realm", description = "JAAS Realm", required = false, multiValued = false)
@Option(name = "--realm", description = "JAAS Realm. Must be combined with --module", required = false, multiValued = false)
Copy link
Member

Choose a reason for hiding this comment

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

That's not fully correct. Realm can be combined with module, but also with index.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why should I combine this with index? Index is already a unique attribute.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, my bad, I changed that. You are right.

@Completion(RealmCompleter.class)
String realmName;

@Option(name = "--index", description = "Realm Index", required = false, multiValued = false)
Copy link
Member

Choose a reason for hiding this comment

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

I would add the "must be combined" statement here and leave realm option description as it is.

@jbonofre jbonofre merged commit 75d79ac into apache:master Aug 10, 2020
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