fix: enforce FoD app/release exclusivity and correct session login options#943
fix: enforce FoD app/release exclusivity and correct session login options#943SangameshV wants to merge 1 commit intodev/v3.xfrom
Conversation
rsenden
left a comment
There was a problem hiding this comment.
We're now duplicating TargetSpecifierArgGroup, AppTarget, ReleaseTarget across three different classes; please move this to an ArgGroup implementation that can be re-used across all three classes, with proper naming like FoDAppOrReleaseArgGroup.
Also, I noticed that you're extending from AbstractFoD*Mixin, but please note that ArgGroups and Mixins are different things, and in general shouldn't be mixed. For example, there might be generic (injection) functionality that operates on Mixins but not ArgGroups, which I guess you already noticed, given the releaseGroup.setDelimiterMixin() call.
We don't want each command class having to explicitly set the delimiter mixin, so please research alternative options, for example extending the injection functionality to ArgGroups, or by having an FoDAppOrReleaseMixin (which itself is not an ArgGroup, but it may contain the exclusive ArgGroup), with this mixin handling the delimiter mixin injection and selection between app vs release.
Currently, the
--appand--releaseoptions are not defined as mutually exclusive in PicoCLI. Instead, they are manually validated, and an error is thrown when both or neither of these options are provided for thefcli fod issue listandfcli fod oss-scan list-componentscommands.This has now been improved by implementing the PicoCLI approach to define these options as mutually exclusive. As a result,
fcli util all-commands list -o jsonwill also correctly reflect these exclusive options.