-
Notifications
You must be signed in to change notification settings - Fork 27
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
Allow running CLI in dev mode, without overwriting quarkus version #1232
Conversation
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 is one only thing (see below). I would prefer to have it changed, but it is not a blocker
.withStream("3.8") | ||
.withPlatformBom(null) | ||
.withManagedResourceCreator( | ||
(serviceContext, quarkusCliClient) -> s -> new CliDevModeVersionLessQuarkusApplicationManagerResource( |
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.
This lambda is a bit hard to comprehend (especially since s parameter is not used). Please, consider, whether this api can be changed to improve readability
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 agree, this lambda is ugly. Thing is, it is two level lambda - basically a reduction for typical problem "how to fix a square thing into round hole". This lambda actually provides instance of ManagedResourceBuilder, which takes just one parameter(ServiceContext) but we need to instantiate CliDevModeLocalhostQuarkusApplicationManagedResource which has two parameters (ServiceContext, QuarkusCliClient).
I tried other approaches like some magic with OOP and interface inheritance or passing just class reference. All of them either hit some java limitation, or have really nasty implementation.
To conclude I found no better way how to solve this, that would not require greater changes in already existing stuff. I at least renamed "s" variable to managedResourceBuilder so it is somehow cleaner what is going on.
…uarkus-qe#1232) * Allow running CLI in dev mode, without overwriting quarkus version
Summary
Create an option for CLI based application to run and not have their quarkus version overwritten. Using up-to-date quarkus is usually a good thing, but for testing CLI stuff like
--stream=3.x
andquarkus update
I want to run the app with the version that is specified in the pom and not overwrite it.This PR should cause no problem to existing stuff, as defaults are the same. It just allows to change this behaviour to run quarkus apps without setting quarkus version as their parameter. Example how to use it is new test.
Please check the relevant options
run tests
phrase in comment)Checklist: