-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
feat(crd-generator): Add CRD-Generator CLI #6337
base: main
Are you sure you want to change the base?
Conversation
97841e9
to
0daccf2
Compare
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.
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.
@manusa Thanks for looking into this.
I added some comments where I'm not sure if it's done right.
8af230d
to
feeb169
Compare
class CRDGeneratorExecutionExceptionHandler implements CommandLine.IExecutionExceptionHandler { | ||
|
||
private static final Logger log = LoggerFactory.getLogger(CRDGeneratorExecutionExceptionHandler.class); | ||
|
||
private final CRDGeneratorCLI crdGeneratorCLI; | ||
private final Supplier<String> diagTextSupplier; |
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.
nitpick: maybe record the result of calling the supplier instead of calling the supplier repeatedly?
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.
The diag text is not yet available when the constructor of CRDGeneratorExecutionExceptionHandler is called. So I have to fetch it somehow later if an error occurs from the CLI class. To prevent a circular reference it's a supplier, so technically cheating 😏. But anyways I think it will be called at most only once: each if statement has a return statement.
crd-generator/cli/src/main/java/io/fabric8/crd/generator/cli/CRDGeneratorCLI.java
Outdated
Show resolved
Hide resolved
… exception handler
Quality Gate passedIssues Measures |
Description
Fixes #5958
Relates to #5950
Add CRD-Generator CLI application (using api-v2).
Usage:
Type of change
test, version modification, documentation, etc.)
Checklist